Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove OTEL/contrib dependency on plugin/storage/es/spanstore/dbmodel #6410

Closed
2 tasks
yurishkuro opened this issue Dec 25, 2024 · 3 comments · Fixed by #6428
Closed
2 tasks

Remove OTEL/contrib dependency on plugin/storage/es/spanstore/dbmodel #6410

yurishkuro opened this issue Dec 25, 2024 · 3 comments · Fixed by #6428
Labels
area/otel area/storage good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Dec 25, 2024 •
edited
Loading

Part of #6408 (please read that first)

logzioexporter depends on plugin/storage/es/spanstore/dbmodel.

This is a completely silly dependency that is easy to remove. I only see it in OTEL/contrib/exporter/logzioexporter/logziospan.go

  • copy whichever code is needed by logziospan.go to OTEL/contrib
  • move plugin/storage/es/spanstore/dbmodel to plugin/storage/es/spanstore/internal/dbmodel
@yurishkuro yurishkuro added good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement labels Dec 25, 2024
@yurishkuro yurishkuro changed the title Remove OTEL/contrib dependency on github.com/jaegertracing/jaeger/plugin/storage/es/spanstore/dbmodel Remove OTEL/contrib dependency on plugin/storage/es/spanstore/dbmodel Dec 25, 2024
@ary82
Copy link

ary82 commented Dec 26, 2024

Hi @yurishkuro, I'm new to the repository. I'm interested in trying this out

@zzzk1
Copy link
Contributor

zzzk1 commented Dec 27, 2024

logziospan.go relies on two factors.

  • /jaeger/model
  • /jaeger/plugin/storage/es/spanstore/dbmodel

we should first copy them to logziospan.go and wait for the changes to be accepted. After that, we can move the dbmodel.go to its new location. @yurishkuro Did I understand it correctly?

@yurishkuro
Copy link
Member Author

yurishkuro commented Dec 27, 2024 •
edited
Loading

no we do not want to cope /model, only the packages specified in the title

This was referenced Dec 27, 2024
Closed
Merged
Manik2708 pushed a commit to Manik2708/jaeger that referenced this issue Jan 5, 2025
@zzzk1 @Manik2708
[chore] Move es/spanstore/dbmodel to internal directory (jaegertracin…
5c5935e 
…g#6428)

- Resolves jaegertracing#6410
- Prerequisite:
open-telemetry/opentelemetry-collector-contrib#36972

- move plugin/storage/es/spanstore/dbmodel to
plugin/storage/es/spanstore/internal/dbmodel

-

- [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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/otel area/storage good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[chore] Move es/spanstore/dbmodel to internal directory zzzk1/jaeger
3 participants
@yurishkuro @zzzk1 @ary82