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

Exporter convenience functions do not set a service name by default #1979

Closed
MrAlias opened this issue Jun 8, 2021 · 2 comments · Fixed by #1985
Closed

Exporter convenience functions do not set a service name by default #1979

MrAlias opened this issue Jun 8, 2021 · 2 comments · Fixed by #1985
Assignees
Labels
pkg:exporter:jaeger Related to the Jaeger exporter package pkg:exporter:otlp Related to the OTLP exporter package pkg:exporter:stdout Related to the stdout exporter package pkg:exporter:zipkin Related to the Zipkin exporter package
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Jun 8, 2021

The InstallNewPipeline and NewExportPipeline create a TracerProvicer that does not have any Resource associated with it. This means that by default the service name, a required attribute, will not be set and the default value will be used. The purpose of these functions was to provide users with a simple one-call solution to setup exporters with all the required values included and production level configuration set. Not setting the service name means these functions are not serving their purpose.

Possible solutions

  1. Remove these functions. They can be useful, but, as this issue points out, their API may not be the most validate with user feedback. Removing these functions prior to a stable release means they can be added back in with more context from measured use.
  2. Add a way for the service name to be passed.
    1. Add an argument for the service name. This will resolve this issue, but will not be extensible.
    2. Add a configuration/options that include the service name as arguments. This will solve this issue, but will be cumbersome and add API bloat.
  3. Leave it as is. These functions will remain incomplete in their setup with best practices but they can be documented that you should not use them if you want your service to be identified, or that you should use environment variables.
@MrAlias MrAlias added release:1.0.0-rc.1 pkg:exporter:otlp Related to the OTLP exporter package pkg:exporter:stdout Related to the stdout exporter package pkg:exporter:jaeger Related to the Jaeger exporter package pkg:exporter:zipkin Related to the Zipkin exporter package labels Jun 8, 2021
@MrAlias MrAlias added this to the RC1 milestone Jun 8, 2021
@seh
Copy link
Contributor

seh commented Jun 8, 2021

I favor removing the functions until we can come up with a better interface. We should be encouraging use of Resources, rather than making it easier to ignore them.

@Aneurysm9
Copy link
Member

I second removing them. While useful, they're also limiting and add API surface that we may not need to carry. I also think they're candidates for later specification/conventionalization when the configuration/convenience WG starts making recommendations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:exporter:jaeger Related to the Jaeger exporter package pkg:exporter:otlp Related to the OTLP exporter package pkg:exporter:stdout Related to the stdout exporter package pkg:exporter:zipkin Related to the Zipkin exporter package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants