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

[jaeger exporter] Document mapping of service name #1222

Merged
merged 5 commits into from
Nov 15, 2020

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Nov 10, 2020

Changes

Documents how service name must be mapped to Jaeger spans.

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@CodeBlanch
Copy link
Member

I'm wondering if we should support Jaeger at all in the spec? It can only detect dependencies when all services in a trace report data into the same instance. Otherwise dependencies are visualized as internal. That works great academically, but shouldn't OpenTelemetry promote trace visualizations with support for real-world enterprise use cases where there will be a heterogeneous mix of instrumented reporting and non-instrumented or non-reporting instrumented services? I'm worried about the support overhead on the SIGs having to explain that and triage issues from confused or mislead users.

Reference: jaegertracing/jaeger-ui#594

@yurishkuro yurishkuro merged commit 8d0d195 into master Nov 15, 2020
@yurishkuro yurishkuro deleted the jaeger-service-name-mapping branch November 15, 2020 00:06
@jkwatson
Copy link
Contributor

If a provided Resource doesn't contain service.* attributes, and service.name in particular, how should the exporter behave? Should the exporter be configured with a fallback serviceName? Or should it just error out on trying to export those spans?

@jkwatson
Copy link
Contributor

#1237

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants