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.

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

[OTEL] Process info is missing from the spans #4534

Closed
Tracked by #3381
yurishkuro opened this issue Jun 17, 2023 · 6 comments · Fixed by #4844 or #4864
Closed
Tracked by #3381

[OTEL] Process info is missing from the spans #4534

yurishkuro opened this issue Jun 17, 2023 · 6 comments · Fixed by #4844 or #4864
Labels
bug help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@yurishkuro
Copy link
Member

When query-service span is generated by Jaeger client, we get some Process information filled out:
image

However, when the spans are produced by OTEL SDK, Process is empty.

image

We need to find out

  • why is the server information not being filled in - doesn't OTEL SDK capture it by default? Maybe we need to enable some resource discovery module?
  • Some attributes produced by OTEL SDK, like otel.library.name, should not be in the span Tags, but in the process. Ideally this should be fixed upstream in OTEL, but we could also implement it internally as a model/adjuster.
@yurishkuro yurishkuro added the bug label Jun 17, 2023
@yurishkuro yurishkuro added the help wanted Features that maintainers are willing to accept but do not have cycles to implement label Jun 17, 2023
@afzal442
Copy link
Contributor

Hey Yuri, I just came over this. how do you think we can adjust Process to have otel.library.name and enable some resource discovery module? Is there any other tags adjusted the same way? How do you see the changes when take this into account? Thanks

@yurishkuro
Copy link
Member Author

adjust Process to have otel.library.name

The simplest way would be to add a new adjuster (under model/adjuster) to Jaeger that would move certain attributes from span.tags to span.process.tags.

enable some resource discovery module

That I don't know, it needs looking into OTEL SDK code.

@NavinShrinivas
Copy link
Contributor

Hey @yurishkuro, I'm looking to making my first contribution to jaeger...This issue seems like a good one, Can I be assigned this? I see it's been a while since this issue was last picked up.

@yurishkuro
Copy link
Member Author

Feel free to pick it up. We do not explicitly assign issues.

@james-ryans
Copy link
Contributor

Sorry... I've encountered the same issue with the previous PR contributor. I've tackled the resource discovery and tag adjuster. Should I remove my tag adjuster and only apply the resource discovery to respect the previous PR contributor?

@NavinShrinivas
Copy link
Contributor

Hey, I was having trouble even getting this off the ground...I think you should go ahead and merge your prs. I'll look into some easier issues to get started with

yurishkuro added a commit that referenced this issue Oct 16, 2023
…pan.Process (#4844)

## Which problem is this PR solving?
- Resolves #4534

## Description of the changes
- Apply OTel resource discovery(detectors) since OTel has plenty of
detectors under the hood. Link
https://pkg.go.dev/go.opentelemetry.io/otel/[email protected]/resource
- Add OTel tags to process tags adjuster and we need to adjust only
`otel.libray.name` and `otel.library.version`. Found here
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/adf5bb501bc65d24ae48088bde37f3c0b13353d0/pkg/translator/jaeger/traces_to_jaegerproto.go#L172
- Bump all OTel semconv to v1.21.0 since telemetrySDK detector use
v1.21.0 semconv.

## How was this change tested?
- Added a new unit test for the adjuster
- Tested manual in local by running `set OTEL_EXPORTER_OTLP_ENDPOINT
http://localhost:4318; go run main.go all --otel-exporter=otlp` and
`make run-all-in-one`, and ordered a ride in HotROD app.
<img width="1440" alt="image"
src="https://github.com/jaegertracing/jaeger/assets/46216691/57220ac9-ec60-40ef-a903-ee815a36dbdf">


## 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`: `yarn lint` and `yarn test`

---------

Signed-off-by: James Ryans <[email protected]>
Signed-off-by: James Ryans <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
yurishkuro added a commit that referenced this issue Oct 19, 2023
## Which problem is this PR solving?
- Resolves #4534
- Continuation from #4844

## Description of the changes
- Add OTel resources to Jaeger components and tracegen tool.

## How was this change tested?
- Tested manually in local
<img width="1440" alt="image"
src="https://github.com/jaegertracing/jaeger/assets/46216691/28bfb121-0c5a-42b1-a3e3-2c5915c90b17">
<img width="1440" alt="image"
src="https://github.com/jaegertracing/jaeger/assets/46216691/3b6d993a-2fac-4c65-97c2-e3dca581a3a1">


## 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`: `yarn lint` and `yarn test`

---------

Signed-off-by: James Ryans <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
4 participants