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/datadog] Deprecate env setting #9017

Merged
merged 4 commits into from
Apr 6, 2022

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Apr 4, 2022

Description:

Deprecate env setting in favor of deployment.environment.

Link to tracking Issue: #9016

Testing: Tested that a deprecation warning is logged when using this setting.

Documentation: Removed usage of env from examples.

@mx-psi mx-psi marked this pull request as ready for review April 4, 2022 11:38
@mx-psi mx-psi requested review from a team and pmm-sumo April 4, 2022 11:38
Copy link
Contributor

@albertvaka albertvaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR looks good to me, but users will have to read the comment in the code or this PR to know what they should use instead of env. Should the warning include the recommended alternative/a link to the docs for deprecated settings?

@mx-psi
Copy link
Member Author

mx-psi commented Apr 6, 2022

The PR looks good to me, but users will have to read the comment in the code or this PR to know what they should use instead of env. Should the warning include the recommended alternative/a link to the docs for deprecated settings?

If an user has a configuration like this:

receivers:
  otlp:
    protocols:
      grpc:

exporters:
  datadog:
    api:
      key: aaaaaaaa
    # ↓↓↓ Using env ↓↓↓
    env: my-custom-env

service:
  telemetry:
    logs:
      level: warn
  pipelines:
    metrics:
      receivers: [otlp]
      exporters: [datadog]

They will get a warning like this:

2022-04-06T14:37:10.044+0200    warn    config/config.go:346    Deprecated: "env" has been deprecated and will be removed in v0.52.0. See https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/9016 {"kind": "exporter", "name": "datadog"}

@@ -406,6 +408,9 @@ func (c *Config) Unmarshal(configMap *config.Map) error {
if c.Version != "" {
c.warnings = append(c.warnings, fmt.Errorf(deprecationTemplate, "version", "v0.52.0", 8783))
}
if c.Env != "" {
c.warnings = append(c.warnings, fmt.Errorf(deprecationTemplate, "env", "v0.52.0", 9016))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(the warning on the previous message comes from here)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is super cool :D

@mx-psi mx-psi added the ready to merge Code review completed; ready to merge by maintainers label Apr 6, 2022
@mx-psi
Copy link
Member Author

mx-psi commented Apr 6, 2022

Marking as ready to merge since albertvaka also works at Datadog

@codeboten codeboten merged commit 57369ff into open-telemetry:main Apr 6, 2022
@mx-psi mx-psi deleted the mx-psi/deprecate-env branch April 7, 2022 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants