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

cli/command: un-export ResourceAttributesEnvvar, DockerCliAttributePrefix #5881

Merged
merged 2 commits into from
Mar 4, 2025

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Mar 3, 2025

relates to:

cli/command: un-export ResourceAttributesEnvvar, DockerCliAttributePrefix

These utility functions were added in 8890a1c,
and are all related to OTEL. The ResourceAttributesEnvvar const defines
the "OTEL_RESOURCE_ATTRIBUTES" environment-variable to use, which is part
of the OpenTelemetry specification, so should be considered a well-known
env-var, and not up to us to define a const for. These code-changes were not
yet included in a release, so we don't have to deprecate.

This patch:

  • Moves the utility functions to the telemetry files, so that all code related
    to OpenTelemetry is together.
  • Un-exports the ResourceAttributesEnvvar to reduce our public API.
  • Un-exports the DockerCliAttributePrefix to reduce depdency on cli/command
    in CLI-plugins, but adds a TODO to move telemetry-related code to a common
    (internal) package.
  • Deprecates the cli-plugins/manager.ResourceAttributesEnvvar const. This
    const has no known consumers, so we could skip deprecation, but just in
    case some codebase uses this.

cli-plugins/manager: move OTEL-related code to separate file

- Human readable description for the release notes

Go SDK: Deprecate `cli-plugins/manager.ResourceAttributesEnvvar` constant. It was used internally, but holds the `OTEL_RESOURCE_ATTRIBUTES` name, which is part of the OpenTelemetry specification. Users of this constant should define their own. It will be removed in the next release.

- A picture of a cute animal (not mandatory but encouraged)

…efix

These utility functions were added in 8890a1c,
and are all related to OTEL. The ResourceAttributesEnvvar const defines
the "OTEL_RESOURCE_ATTRIBUTES" environment-variable to use, which is part
of the [OpenTelemetry specification], so should be considered a well-known
env-var, and not up to us to define a const for. These code-changes were not
yet included in a release, so we don't have to deprecate.

This patch:

- Moves the utility functions to the telemetry files, so that all code related
  to OpenTelemetry is together.
- Un-exports the ResourceAttributesEnvvar to reduce our public API.
- Un-exports the DockerCliAttributePrefix to reduce depdency on cli/command
  in CLI-plugins, but adds a TODO to move telemetry-related code to a common
  (internal) package.
- Deprecates the cli-plugins/manager.ResourceAttributesEnvvar const. This
  const has no known consumers, so we could skip deprecation, but just in
  case some codebase uses this.

[OpenTelemetry specification]: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added this to the 28.0.2 milestone Mar 3, 2025
@thaJeztah thaJeztah self-assigned this Mar 3, 2025
@thaJeztah thaJeztah requested review from jsternberg and Benehiko March 3, 2025 13:29
@codecov-commenter
Copy link

codecov-commenter commented Mar 3, 2025

Codecov Report

Attention: Patch coverage is 58.20896% with 28 lines in your changes missing coverage. Please review.

Project coverage is 59.34%. Comparing base (43a2fcf) to head (8bedb69).
Report is 81 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5881      +/-   ##
==========================================
+ Coverage   59.31%   59.34%   +0.02%     
==========================================
  Files         353      354       +1     
  Lines       29735    29735              
==========================================
+ Hits        17637    17645       +8     
+ Misses      11117    11108       -9     
- Partials      981      982       +1     
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thaJeztah thaJeztah added the impact/go-sdk Noteworthy (compatibility changes) in the Go SDK label Mar 3, 2025
@thaJeztah thaJeztah requested a review from vvoland March 4, 2025 12:28
Copy link
Member

@Benehiko Benehiko left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 0b985e7 into docker:master Mar 4, 2025
110 of 116 checks passed
@thaJeztah thaJeztah deleted the cleanup_otel branch March 4, 2025 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics/otel area/metrics impact/deprecation impact/go-sdk Noteworthy (compatibility changes) in the Go SDK kind/refactor PR's that refactor, or clean-up code status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants