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

Upgrade Query API v3 to official OpenTelemetry format #4648

Closed
3 tasks done
yurishkuro opened this issue Aug 12, 2023 · 5 comments · Fixed by #4742
Closed
3 tasks done

Upgrade Query API v3 to official OpenTelemetry format #4648

yurishkuro opened this issue Aug 12, 2023 · 5 comments · Fixed by #4742
Labels
enhancement

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Aug 12, 2023 •
edited
Loading

The quest service has api/v3 endpoint that returns data in the OpenTelemetry format. However, it uses pre-1.0 version of OTLP, in particular the InstrumentationLibrarySpan container message instead of ScopedSpans. It also performs manual translation from Jaeger model to OTEL, even though the translation modules are available in OTEL contrib.

Proposal:

  • update jaeger-idl to the current version of OTLP
  • change codegen in jaeger to use the new IDL (see Makefile)
  • modify the package to use OTEL translator instead of manual translation
@yurishkuro yurishkuro added enhancement help wanted Features that maintainers are willing to accept but do not have cycles to implement labels Aug 12, 2023
@yurishkuro yurishkuro mentioned this issue Aug 12, 2023
Closed
@tanayvaswani
Copy link

Do we need to implement model2otel auto translator for all the five methods i.e jaegerSpansToOTLP, jSpanToOTLP, jTagsToOTLP, jLogsToOTLP, jReferencesToOTLP ?

@yurishkuro
Copy link
Member Author

the goal of the ticket is to remove implementations, by reusing OTEL's translator.

@tanayvaswani
Copy link

Sorry, I shouldn't have used the word "implement", I will try working on it, thanks. :)

@tanayvaswani
Copy link

Aren't we already using ResourceSpans which seems to be a part of current version of OTLP ?
Ref: here
The ResourceSpans says that "A collection of ScopeSpans from a Resource".

I don't know if I am referring something which is not accurate, I'm new to Jaeger & OpenTelemetry, I have the willingness to solve this issue.

@yurishkuro
Copy link
Member Author

jaeger/proto-gen/otel/trace/v1/trace.pb.go

Line 171 in 6b7f073

InstrumentationLibrarySpans []*InstrumentationLibrarySpans `protobuf:"bytes,2,rep,name=instrumentation_library_spans,json=instrumentationLibrarySpans,proto3" json:"instrumentation_library_spans,omitempty"`

@tanayvaswani tanayvaswani mentioned this issue Aug 17, 2023
Closed
4 tasks
@yurishkuro yurishkuro mentioned this issue Sep 10, 2023
Merged
@yurishkuro yurishkuro removed the help wanted Features that maintainers are willing to accept but do not have cycles to implement label Sep 10, 2023
yurishkuro added a commit that referenced this issue Sep 10, 2023
@yurishkuro
Upgrade Query API v3 to official OpenTelemetry format (#4742)
8f680cc 
## Which problem is this PR solving?
- Resolves #4648
- Closes #4682 

## Description of the changes
- pull the latest version of jaeger-idl that uses OTLP v1
- regenerate protobuf types
- remove explicit translation with
`github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger`
- fix tests. Unfortunately, the actual translation returns error that
cannot be reproduced / tested, thus coverage declined a bit

## How was this change tested?
- CI only

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Upgrade Query API v3 to official OpenTelemetry format yurishkuro/jaeger
2 participants
@yurishkuro @tanayvaswani