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

otelconfig: new module as a copy of config module which is being deprecated #6796

Merged

Conversation

mikeblum
Copy link
Contributor

@mikeblum mikeblum commented Feb 17, 2025

Towards #5597

  • Add go.opentelemetry.io/contrib/otelconf module which is a replacement for go.opentelemetry.io/contrib/config.
  • Deprecate go.opentelemetry.io/contrib/config module in favor of go.opentelemetry.io/contrib/otelconf. This is the last release of this module.

@mikeblum mikeblum marked this pull request as ready for review February 17, 2025 18:47
@mikeblum mikeblum requested a review from a team as a code owner February 17, 2025 18:47
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

We should not remove the existing module but keep it for one release to give the users time to migrate.

Related comment: #5597 (comment)

@mikeblum mikeblum force-pushed the gh-5597-rename-config-to-otelconf branch 4 times, most recently from 7b87b23 to f7257fa Compare February 19, 2025 03:39
@mikeblum mikeblum requested a review from pellared February 19, 2025 03:40
@mikeblum mikeblum force-pushed the gh-5597-rename-config-to-otelconf branch from bd3f672 to f084515 Compare February 19, 2025 20:57
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

This looks closer to the desired solution.

@pellared
Copy link
Member

I encourage checking with pkgsite instead of godoc. See: https://github.com/open-telemetry/opentelemetry-go/blob/main/CONTRIBUTING.md#documentation

@MrAlias MrAlias added this to the v1.35.0 milestone Feb 20, 2025
@pellared
Copy link
Member

@mikeblum, make sure to not lose recent changes of config module: https://github.com/open-telemetry/opentelemetry-go-contrib/commits/main/config

@mikeblum mikeblum force-pushed the gh-5597-rename-config-to-otelconf branch from f084515 to 68c1aee Compare February 20, 2025 18:10
@mikeblum
Copy link
Contributor Author

mikeblum commented Feb 20, 2025

❯ diff config otelconf
diff --color config/doc.go otelconf/doc.go
4,9c4
< // Package config is deprecated.
< //
< // Deprecated: use [go.opentelemetry.io/contrib/otelconf] instead.
< // This is the last release of this module.
< //
< // Package config can be used to parse a configuration file that follows
---
> // Package otelconf can be used to parse a configuration file that follows
13c8
< // go.opentelemetry.io/contrib/config/v0.3.0 includes code that supports the
---
> // go.opentelemetry.io/contrib/otelconf/v0.3.0 includes code that supports the
15c10
< package config // import "go.opentelemetry.io/contrib/config"
---
> package otelconf // import "go.opentelemetry.io/contrib/otelconf"
diff --color config/go.mod otelconf/go.mod
1,2c1
< // Deprecated: use [go.opentelemetry.io/contrib/otelconf] instead.
< module go.opentelemetry.io/contrib/config
---
> module go.opentelemetry.io/contrib/otelconf
Common subdirectories: config/testdata and otelconf/testdata
Common subdirectories: config/v0.2.0 and otelconf/v0.2.0
Common subdirectories: config/v0.3.0 and otelconf/v0.3.0

is the current diff between the two modules as of:

commit 39ff1c57e213e67753fa7f72a51c22bc1b4a7cba (upstream/main, main)
Author: Damien Mathieu <[email protected]>
Date:   Thu Feb 20 18:55:31 2025 +0100

    Add http semantic conventions notice to changelog (#6737)

@MrAlias
Copy link
Contributor

MrAlias commented Feb 20, 2025

SIG meeting notes:

Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

nit comments only.
Thanks on working on this.

@dmathieu
Copy link
Member

The Prometheus PR has been merged. You can now apply its changes to your PR.

Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

Just waiting for "sync" 👍

@mikeblum mikeblum force-pushed the gh-5597-rename-config-to-otelconf branch 2 times, most recently from a3ae741 to d936a53 Compare February 21, 2025 14:47
@mikeblum mikeblum requested a review from pellared February 21, 2025 14:49
@mikeblum
Copy link
Contributor Author

mikeblum commented Feb 21, 2025

cc @pellared

  • sync 🔁 complete ✅ - fetched upstream/main and pulled in the prom changes. diff report is below 👇
  • do i need to squash / rebase down to a single commit locally first or can we squash in the GH UI? Didn't want to wipe out the commit log during review. squash happens on merge.
❯ diff config otelconf
diff --color config/doc.go otelconf/doc.go
4,9c4
< // Package config is deprecated.
< //
< // Deprecated: use [go.opentelemetry.io/contrib/otelconf] instead.
< // This is the last release of this module.
< //
< // Package config can be used to parse a configuration file that follows
---
> // Package otelconf can be used to parse a configuration file that follows
13c8
< // go.opentelemetry.io/contrib/config/v0.3.0 includes code that supports the
---
> // go.opentelemetry.io/contrib/otelconf/v0.3.0 includes code that supports the
15c10
< package config // import "go.opentelemetry.io/contrib/config"
---
> package otelconf // import "go.opentelemetry.io/contrib/otelconf"
diff --color config/go.mod otelconf/go.mod
1,3c1
< // Deprecated: use go.opentelemetry.io/contrib/otelconf instead.
< // This is the last release of this module.
< module go.opentelemetry.io/contrib/config
---
> module go.opentelemetry.io/contrib/otelconf
Common subdirectories: config/testdata and otelconf/testdata
Common subdirectories: config/v0.2.0 and otelconf/v0.2.0
Common subdirectories: config/v0.3.0 and otelconf/v0.3.0

@dmathieu
Copy link
Member

No need to squash. Merging the PR will do that automatically.

@mikeblum mikeblum force-pushed the gh-5597-rename-config-to-otelconf branch from d936a53 to f744694 Compare February 21, 2025 17:06
@pellared pellared changed the title refactor(config): refactor contrib/config package to contrib/otelconf New otelconfig moduleas a copy of config module which is being deprecated Feb 21, 2025
@pellared pellared changed the title New otelconfig moduleas a copy of config module which is being deprecated New otelconfig module as a copy of config module which is being deprecated Feb 21, 2025
@pellared pellared changed the title New otelconfig module as a copy of config module which is being deprecated otelconfig: New module as a copy of config module which is being deprecated Feb 21, 2025
@pellared pellared changed the title otelconfig: New module as a copy of config module which is being deprecated otelconfig: new module as a copy of config module which is being deprecated Feb 21, 2025
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

More changes need to be applied in otelconf (tests do not pass).

Copy link

codecov bot commented Feb 22, 2025

Codecov Report

Attention: Patch coverage is 81.35033% with 395 lines in your changes missing coverage. Please review.

Project coverage is 76.1%. Comparing base (74e6d32) to head (a128e28).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
otelconf/v0.3.0/config_json.go 49.2% 96 Missing and 47 partials ⚠️
otelconf/v0.2.0/generated_config.go 43.5% 70 Missing and 35 partials ⚠️
otelconf/v0.3.0/metric.go 89.4% 34 Missing and 10 partials ⚠️
otelconf/v0.2.0/metric.go 88.7% 34 Missing and 8 partials ⚠️
otelconf/v0.3.0/config_yaml.go 62.5% 12 Missing and 6 partials ⚠️
otelconf/v0.2.0/config.go 80.3% 8 Missing and 4 partials ⚠️
otelconf/v0.3.0/config.go 89.7% 7 Missing and 3 partials ⚠️
otelconf/v0.3.0/log.go 95.5% 4 Missing and 3 partials ⚠️
otelconf/v0.2.0/log.go 94.3% 3 Missing and 3 partials ⚠️
otelconf/v0.2.0/trace.go 97.1% 2 Missing and 2 partials ⚠️
... and 1 more
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #6796     +/-   ##
=======================================
+ Coverage   75.5%   76.1%   +0.5%     
=======================================
  Files        206     219     +13     
  Lines      19195   21313   +2118     
=======================================
+ Hits       14498   16224   +1726     
- Misses      4259    4529    +270     
- Partials     438     560    +122     
Files with missing lines Coverage Δ
config/v0.2.0/config.go 80.3% <ø> (ø)
config/v0.3.0/config.go 89.7% <ø> (ø)
otelconf/v0.2.0/resource.go 100.0% <100.0%> (ø)
otelconf/v0.3.0/resource.go 100.0% <100.0%> (ø)
otelconf/v0.2.0/trace.go 97.1% <97.1%> (ø)
otelconf/v0.3.0/trace.go 97.4% <97.4%> (ø)
otelconf/v0.2.0/log.go 94.3% <94.3%> (ø)
otelconf/v0.3.0/log.go 95.5% <95.5%> (ø)
otelconf/v0.3.0/config.go 89.7% <89.7%> (ø)
otelconf/v0.2.0/config.go 80.3% <80.3%> (ø)
... and 5 more

... and 1 file with indirect coverage changes

@pellared pellared merged commit 6f9495e into open-telemetry:main Feb 22, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: file-config Related to file-based configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants