[ES][v2] Implement WriteTraces for v2
#7020
Conversation
Signed-off-by: Manik2708 <[email protected]>
|
|
||
| // WriteTraces convert the traces to ES Span model and write into the database | ||
| func (t *TraceWriter) WriteTraces(_ context.Context, td ptrace.Traces) error { | ||
| dbSpans := ToDBModel(td) |
There was a problem hiding this comment.
Once tagDotReplacement is embedded, this must be replaced by lambda function
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7020 +/- ##
==========================================
+ Coverage 95.99% 96.01% +0.01%
==========================================
Files 346 347 +1
Lines 20622 20637 +15
==========================================
+ Hits 19797 19815 +18
+ Misses 622 620 -2
+ Partials 203 202 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| dbSpans := ToDBModel(td) | ||
| for i := 0; i < len(dbSpans); i++ { | ||
| span := &dbSpans[i] | ||
| t.spanWriter.WriteSpan(model.EpochMicrosecondsAsTime(span.StartTime), span) |
There was a problem hiding this comment.
I think this is where we should start diverging with v1 implementation. One of the strengths of v2 storage API is that it allows batch writes. v1 API was a single span at a time. So v1 writer had a workaround by running a background batch insert, which also had a problem of not being able to report errors from the writes.
I think we can do better in v2. I don't know if we even need to have a background batching, because this can already configured in the pipeline above the storage. Instead we can treat each payload as already large-enough batch and write all spans in a single request.
| return nil | ||
| } | ||
|
|
||
| func (t *TraceWriter) CreateTemplates(spanTemplate, serviceTemplate string, indexPrefix cfg.IndexPrefix) error { |
There was a problem hiding this comment.
why is this a public function? Who would be calling it?
| coreWriter.On("Close").Return(nil) | ||
| writer := TraceWriter{spanWriter: coreWriter} | ||
| err := writer.Close() | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
one-sided test. Wrap it in a loop for _, expErr := range []error{nil, assert.AnError} and check that correct value is returned from Close.
yurishkuro
added
the
changelog:exprimental
f2f4479
## Which problem is this PR solving? - Fixes a part of: jaegertracing#6458 ## Description of the changes - Implement the `WriteTraces` method for v2 ## How was this change tested? - Unit Tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` Signed-off-by: Manik2708 <[email protected]>
## Which problem is this PR solving? - Fixes a part of: jaegertracing#6458 ## Description of the changes - Implement the `WriteTraces` method for v2 ## How was this change tested? - Unit Tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` Signed-off-by: Manik2708 <[email protected]> Signed-off-by: amol-verma-allen <[email protected]>
Which problem is this PR solving?
Description of the changes
WriteTracesmethod for v2How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test