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

[cmd/telemetrygen] Use standard SDK configuration #35736

Open
axw opened this issue Oct 11, 2024 · 7 comments
Open

[cmd/telemetrygen] Use standard SDK configuration #35736

axw opened this issue Oct 11, 2024 · 7 comments
Labels
cmd/telemetrygen telemetrygen command enhancement New feature or request Stale

Comments

@axw
Copy link
Contributor

axw commented Oct 11, 2024

Component(s)

cmd/telemetrygen

Is your feature request related to a problem? Please describe.

Right now telemetrygen has various non-standard flags for configuring

  • OTLP exporter: --otlp-endpoint, --otlp-header, --mtls, etc.
  • SDK processors: --batch
  • Resource attributes: --otlp-attributes, --service

Having non-standard configuration adds some cognitive overhead for users of the tool, as they must learn about the tool's specific configuration.

Describe the solution you'd like

I would like telemetrygen to support standard OTel SDK configuration: https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main/config. That way end users have a single standard configuration format for the things described above.

When adding support for standard config, deprecate existing telemetrygen flags (except those that are specific to telemetrygen logic), and remove after a grace period.

Describe alternatives you've considered

Use https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main/exporters/autoexport, which supports a standard set of env vars for configuring the SDK: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/. This approach is being replaced by the config file approach mentioned above.

No response

Additional context

No response

@axw axw added enhancement New feature or request needs triage New item requiring triage labels Oct 11, 2024
@github-actions github-actions bot added the cmd/telemetrygen telemetrygen command label Oct 11, 2024
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@mx-psi
Copy link
Member

mx-psi commented Oct 11, 2024

If we go down this route, should we support the OTel SDK configuration file instead? This is meant to be the replacement for environment variables

@atoulme atoulme removed the needs triage New item requiring triage label Oct 12, 2024
@axw
Copy link
Contributor Author

axw commented Oct 13, 2024

@mx-psi sounds sensible. I'll rename the issue to be less about env vars, and more about "standard SDK configuration". What I really intended was for telemetrygen to follow an SDK standard.

I had a read of the config repo & OTEP, and couldn't find a clear answer: will there be no standard env vars in the long run? I certainly see the value in having a config file format, and understand that env vars are not expressive enough for all configuration, but I think there's also value in having standard default config that supports the current env vars for simple things such as the OTLP endpoint.

Has there been any discussion of having a default, built-in config in SDKs, e.g. https://github.com/open-telemetry/opentelemetry-configuration/blob/main/examples/sdk-migration-config.yaml?

Any pointers to relevant docs/specs/discussions would be appreciated.

@axw axw changed the title [cmd/telemetrygen] Support OTLP exporter with environment variables [cmd/telemetrygen] Use standard SDK configuration Oct 13, 2024
@mx-psi
Copy link
Member

mx-psi commented Oct 14, 2024

@codeboten can probably answer these questions better than I can, but I believe this is meant to be (once stable) the recommended way of configuring the SDK. OTEP 225 has probably useful context

@axw
Copy link
Contributor Author

axw commented Oct 15, 2024

@dmathieu pointed out to me that the environment variable specification is stable, so removing/breaking them would require a major version bump.

@mx-psi
Copy link
Member

mx-psi commented Oct 15, 2024

@axw Yes, the env var spec would still be supported, what I meant with the recommended way was the way that is used in all examples and documentation for example.

Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd/telemetrygen telemetrygen command enhancement New feature or request Stale
Projects
None yet
Development

No branches or pull requests

3 participants