[exporter] Remove jaeger dbmodel dependency #36972
Conversation
## Which problem is this PR solving? - Resolves #6410 - Prerequisite: open-telemetry/opentelemetry-collector-contrib#36972 ## Description of the changes - move plugin/storage/es/spanstore/dbmodel to plugin/storage/es/spanstore/internal/dbmodel ## How was this change tested? - ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] 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: zzzk1 <[email protected]>
| } | ||
|
|
||
| // LogzioSpan is same as ESSpan with a few different json field names and an addition on type field. | ||
| type LogzioSpan struct { |
There was a problem hiding this comment.
none of the new structs should public
There was a problem hiding this comment.
@zzzk1 I agree with @yurishkuro
|
@fatsheep9146 @yurishkuro @yotamloe Improved, please take a look 😊 |
zzzk1
force-pushed
the
remove-dbmodel
branch
from
344199d to
c575410
Compare
| } | ||
|
|
||
| // only for testing span is ES database representation of the domain span. | ||
| type span struct { |
There was a problem hiding this comment.
why is this type needed? why not use logziospan?
There was a problem hiding this comment.
why is this type needed? why not use logziospan?
unit test fromDomain_test.go needs this struct to compare
| Tag map[string]any `json:"JaegerTag,omitempty"` | ||
| Logs []spanLog `json:"logs"` | ||
| Process process `json:"process,omitempty"` | ||
| Type string `json:"type"` | ||
| } |
There was a problem hiding this comment.
still unnecessary changes to the type - why change the field order?
There was a problem hiding this comment.
also, since you need to reference moved types like TraceID, Process, etc., you can define them in internal/dbmodel package, where they can remain public but not usable outside of this exporter, and the code referring to them will not need to change, just the import.
| ) | ||
|
|
||
| // Span is ES database representation of the domain span. | ||
| type Span struct { |
There was a problem hiding this comment.
@yurishkuro func TestFromDomainEmbedProcess(t *testing.T) needs this struct
There was a problem hiding this comment.
why can't that test use logziospan?
| @@ -7,7 +7,8 @@ import ( | |||
| "encoding/json" | |||
There was a problem hiding this comment.
I would move this file to internal/dbmodel as well
Description
Copy the Jaeger plugin/storage/es/spanstore/dbmodel for logzioexporter, and remove the dependency. The dbmodel package in Jaeger will be internal from v1.65 (jaegertracing/jaeger#6428).
Link to tracking issue
Fixes #36970
Testing
Covered by existing