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

Service name should be mandatory for all configured resources. #1241

Closed
jkwatson opened this issue Nov 19, 2020 · 18 comments · Fixed by #1269
Closed

Service name should be mandatory for all configured resources. #1241

jkwatson opened this issue Nov 19, 2020 · 18 comments · Fixed by #1269
Assignees
Labels
area:sdk Related to the SDK priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:resource Related to the specification/resource directory

Comments

@jkwatson
Copy link
Contributor

What are you trying to achieve?

The resource specifications currently allow the entire service.* namespace to be missing from a Resource configured into the SDK. This means that exporters that require a service.name must be configured (or hard-coded) with a fallback name. Since all services should have a service.name, we should simply require that to be set on any resource that is attached to an SDK.

Additional context.

This came up via the following spec clarification request for how the Jaeger exporter should behave in the absence of a service.name in the Resource: #1237

@jkwatson jkwatson added the spec:resource Related to the specification/resource directory label Nov 19, 2020
@yurishkuro
Copy link
Member

+1

@yurishkuro
Copy link
Member

@jkwatson this page already says that the service.name attribute is required on the service resource.

However, where in the spec a Resource is even associated with the span or tracer?

@jkwatson
Copy link
Contributor Author

@jkwatson this page already says that the service.name attribute is required on the service resource.

But, a service resource is not required, so you don't necessarily get a service.name.

However, where in the spec a Resource is even associated with the span or tracer?

This should be in the SDK spec somewhere.

@anuraaga
Copy link
Contributor

Getting OTEL_SERVICE_NAME as a part of this conversation would be a huge win for users. I'm seeing a lot of people tripping up over the magic OTEL_RESOURCE_ATTRIBUTES=service.name=catsservice incantation, this feels neither required nor intuitive.

@Oberon00 Oberon00 added the area:sdk Related to the SDK label Nov 19, 2020
@Oberon00
Copy link
Member

I think it would be best to require all SDKs to provide a reasonable default service name to exporters if there is none explicitly configured. For example, use the same as process.executable.name (which can be auto-detected), use the main class name or the main script name.

@andrewhsu andrewhsu added priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA labels Nov 20, 2020
@andrewhsu
Copy link
Member

@tedsuo fyi

@carlosalberto
Copy link
Contributor

From the SIG today: as many languages already provide fallback/default values for service.name, we will have a PR with this approach. The actual value will be language-specific and will be based on the actual running process info.

@yurishkuro
Copy link
Member

I would still propose a change to the spec saying:

  • service resource with service name is required
  • if it is not supplied, the SDK must use language specific means of inferring a service name
  • if that mechanism fails as well, the SDK must create a service resource with service name unknown_service

Side note: I am somewhat skeptical about step 2. For example, in Java the binary name is java, and the startup class name could be a generic framework class. The mechanism must be smart enough to recognize those cases and not use generic names. In addition, using a framework specific name is not ideal either, since it could be different from what is known as "service" to the orchestration, service discovery, and routing infrastructure. It's the latter that's the most useful as the service name for telemetry.

@Oberon00
Copy link
Member

The mechanism must be smart enough to recognize those cases and not use generic names.

I think a generic framework name is still better than "unknown_service" so while it would be nice if it was smarter, I don't think its very important.

@yurishkuro
Copy link
Member

I think a generic framework name is still better than "unknown_service"

perhaps you're right

@jkwatson
Copy link
Contributor Author

jkwatson commented Jan 5, 2021

Re-opening this, as it has been made less clear over time.
See #1294

@jkwatson jkwatson reopened this Jan 5, 2021
@Oberon00
Copy link
Member

Oberon00 commented Jan 5, 2021

With the discussion around #1294, this is now my opinion:

  • There should not be any resource attribute that is mandatory, not even service name.
  • However, there should be a simple and obvious fallback service.name for exporters that need it. For example, by providing something like Resource.getDefault().getStringAttribute("service.name").

@carlosalberto
Copy link
Contributor

@Oberon00 @jkwatson after #1294 was merged, is there anything left to be done or clarified here?

@jkwatson
Copy link
Contributor Author

@Oberon00 @jkwatson after #1294 was merged, is there anything left to be done or clarified here?

seems fine to me. thanks!

@carlosalberto
Copy link
Contributor

Ping @Oberon00 ;)

@Oberon00
Copy link
Member

I think we should specify how/when Jaeger/Zipkin should use the default resource's attribute.

@carlosalberto
Copy link
Contributor

@Oberon00 Would mind elaborating? (else, you could help cook a PR yourself if you have enough cycles this week).

@Oberon00
Copy link
Member

Oh, actually there is a separate issue for that: #1237. I think I'll close this then, since I was the only one objecting to closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:resource Related to the specification/resource directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants