update config dependency #11611

codeboten · 2024-11-05T21:12:08Z

This PR does a couple of things that I couldn't quite split up so I put together a PR w/ individual commits to help reviewers get through it. This PR does the following:

update go.opentelemetry.io/contrib/config package to latest. this brings in breaking changes. in order to prevent those breaking changes from impacting end users, i've also added a layer of config unmarshaling
updates the collector to instantiate the meter provider (and exporters) via the config package. this allows us to remove all the code in otelinit. the reason for including this change was that unmarshaling the config was causing circular dependencies i didn't want to address by moving code that could be deleted around.

Replacement for #11458.

Fixes #12021

mjnowen · 2024-11-12T09:27:31Z

Looking forward to seeing this get merged!

github-actions · 2024-11-27T03:22:40Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

mjnowen · 2024-11-27T09:09:08Z

Not stale? @codeboten

codecov · 2024-12-09T19:13:31Z

Codecov Report

Attention: Patch coverage is 86.12100% with 39 lines in your changes missing coverage. Please review.

Project coverage is 91.64%. Comparing base (a3c7d95) to head (40d3021).

Files with missing lines	Patch %	Lines
service/telemetry/internal/migration/v0.3.0.go	54.54%	12 Missing and 3 partials ⚠️
service/telemetry/internal/migration/v0.2.0.go	93.67%	8 Missing and 2 partials ⚠️
service/service.go	84.90%	6 Missing and 2 partials ⚠️
component/componenttest/configtest.go	0.00%	2 Missing and 1 partial ⚠️
service/telemetry/metrics.go	60.00%	1 Missing and 1 partial ⚠️
service/telemetry/config.go	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11611      +/-   ##
==========================================
- Coverage   91.67%   91.64%   -0.03%     
==========================================
  Files         461      462       +1     
  Lines       24749    24744       -5     
==========================================
- Hits        22689    22677      -12     
- Misses       1678     1681       +3     
- Partials      382      386       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codeboten · 2024-12-10T00:38:41Z

Could use some feedback from @open-telemetry/collector-approvers on this latest commit to the PR to update the config support from v0.2.0 to v0.3.0.

tl;dr there's a backwards incompatible change in how headers are passed in and I wanted to support users transitioning

i wanted to support both the 0.3.0 and the 0.2.0 schema, which resulted in some migration code. I still have a few tests to add, and this change will need a PR in go-contrib to be merged before it can be completely done. Anyways, i could use your input: df83265

This is waiting on open-telemetry/opentelemetry-go-contrib#6412

dmitryax · 2024-12-10T01:06:00Z

The approach to handle the backward incompatible change looks good to me 👍

dmitryax · 2024-12-10T01:07:16Z

service/telemetry/internal/migration/v0.3.0.go

+			// compatibility attempts
+			return err
+		}
+		// TODO: add a warning log to tell users to migrate their config


I guess it might be complicated to pass a logger here, but it's pretty important. I think we should come up with a solution by the time we release this change

The way we do this on the Datadog exporter is to have a private field that is written to during unmarshaling and then a method to log warnings.

I don't love it, but I haven't been able to come up with anything better

jpkrohling

I appreciate your efforts to not break our current users. And explicitly about this:

Anyways, i could use your input: df83265

I like your approach. We don't have a schema version anywhere, so trying to fallback to the previous version before failing is a good approach. It doesn't scale nicely, but I also don't see a good way to determine the version other than requiring users to provide a schema version (which is bad UX for this specific place of the config, IMO).

jpkrohling · 2024-12-10T08:42:45Z

service/service.go

@@ -363,3 +385,43 @@ func pdataFromSdk(res *sdkresource.Resource) pcommon.Resource {
 	}
 	return pcommonRes
 }
+
+func disableHighCardinalityMetrics() []config.View {


looks OK for now, but if we expect this list to grow before we remove the gate, it might make sense to externalize this somehow?

This is the purpose of #11754

service/service.go

mx-psi

I think this is a good opportunity to resolve #10808 by writing down what our plans are regarding this.

mx-psi · 2024-12-10T11:40:28Z

service/telemetry/internal/migration/v0.3.0.go

+			// compatibility attempts
+			return err
+		}
+		// TODO: add a warning log to tell users to migrate their config


The way we do this on the Datadog exporter is to have a private field that is written to during unmarshaling and then a method to log warnings.

I don't love it, but I haven't been able to come up with anything better

mx-psi · 2024-12-10T11:41:37Z

service/service.go

@@ -363,3 +385,43 @@ func pdataFromSdk(res *sdkresource.Resource) pcommon.Resource {
 	}
 	return pcommonRes
 }
+
+func disableHighCardinalityMetrics() []config.View {


This is the purpose of #11754

github-actions · 2025-01-01T03:19:18Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

This change updates the dependency on go.opentelemetry.io/contrib/config to the latest in main. This is because v0.11.0 has a bug that prevents resource attributes to be applied as labels in the prometheus exporter. This change contains the following pieces: 1. the update of the dependency 2. add a layer to prevent breaking end users' configuration of exporter headers 3. move the meter provider initialization to depend on the config package instead of otelinit Number 3 didn't need to happen in this PR, but doing #2 required a bunch of changes that can be avoided by moving to using the meter provider from config instead of otelinit. Signed-off-by: Alex Boten <[email protected]>

Signed-off-by: Alex Boten <[email protected]>

This allows end users to configure their internal metrics export with either the new v0.3.0 schema or the existing v0.2.0 schema. Signed-off-by: Alex Boten <[email protected]>

Signed-off-by: Alex Boten <[email protected]>

codeboten force-pushed the codeboten/update-config-dependency branch 3 times, most recently from 6f225b7 to 47f403a Compare November 6, 2024 21:45

codeboten mentioned this pull request Nov 19, 2024

wip: update config dependency #11458

Closed

github-actions bot added the Stale label Nov 27, 2024

codeboten removed the Stale label Nov 27, 2024

codeboten force-pushed the codeboten/update-config-dependency branch from 47f403a to 2a0e6f8 Compare December 9, 2024 19:03

dmitryax reviewed Dec 10, 2024

View reviewed changes

jpkrohling approved these changes Dec 10, 2024

View reviewed changes

mx-psi reviewed Dec 10, 2024

View reviewed changes

codeboten force-pushed the codeboten/update-config-dependency branch 4 times, most recently from 3bcf5f1 to b0021fb Compare December 16, 2024 23:07

codeboten marked this pull request as ready for review December 16, 2024 23:07

codeboten requested a review from a team as a code owner December 16, 2024 23:07

codeboten requested a review from atoulme December 16, 2024 23:07

codeboten force-pushed the codeboten/update-config-dependency branch from 41beb42 to e30a736 Compare December 17, 2024 19:44

jade-guiton-dd mentioned this pull request Dec 20, 2024

Change internal logs config example to use OTLP/HTTP open-telemetry/opentelemetry.io#5831

Merged

codeboten mentioned this pull request Dec 21, 2024

Update All opentelemetry-go-contrib packages #11923

Closed

1 task

github-actions bot added the Stale label Jan 1, 2025

codeboten removed the Stale label Jan 6, 2025

jaronoff97 mentioned this pull request Jan 6, 2025

Mismatch in resource attributes between metrics and everything else #12021

Open

codeboten mentioned this pull request Jan 9, 2025

Support unix socket for OTLP with gRPC export of internal telemetry #12059

Closed

codeboten added 24 commits January 17, 2025 13:04

update MetricExporter to PullMetricExporter in various tests

5ea782b

Signed-off-by: Alex Boten <[email protected]>

update attributes func to return slice of AttributeNameValue

fea29f1

Signed-off-by: Alex Boten <[email protected]>

update factory to pass in context

2a7643b

Signed-off-by: Alex Boten <[email protected]>

remove otelinit

5a6b4d5

Signed-off-by: Alex Boten <[email protected]>

update metrics test

f5a48a2

Signed-off-by: Alex Boten <[email protected]>

set meter provider config in SDK initialization

3c06aaf

Signed-off-by: Alex Boten <[email protected]>

fix test utility to return a valid ipv6 address and prom configuration

6800c8c

Signed-off-by: Alex Boten <[email protected]>

fix attributes test

97b4112

Signed-off-by: Alex Boten <[email protected]>

move code to disable high cardinality to use views in service

7fb2a72

Signed-off-by: Alex Boten <[email protected]>

add changelog

d766f65

Signed-off-by: Alex Boten <[email protected]>

ensure metric level none disables metric readers

c058120

Signed-off-by: Alex Boten <[email protected]>

ensure the sdk is shutdown on error

4571447

Signed-off-by: Alex Boten <[email protected]>

fix service_test

a1ac93a

Signed-off-by: Alex Boten <[email protected]>

add migration from v0.2.0 to v0.3.0 support

c24793f

This allows end users to configure their internal metrics export with either the new v0.3.0 schema or the existing v0.2.0 schema. Signed-off-by: Alex Boten <[email protected]>

add alias

aa44b14

Signed-off-by: Alex Boten <[email protected]>

add v0.3.0 tests

8156352

Signed-off-by: Alex Boten <[email protected]>

update imports

bb1c9eb

Signed-off-by: Alex Boten <[email protected]>

fix additional imports

df5c512

Signed-off-by: Alex Boten <[email protected]>

fix package statements

13d3202

Signed-off-by: Alex Boten <[email protected]>

update test to reflect error message

6ca3722

Signed-off-by: Alex Boten <[email protected]>

ignore AdditionalProperties when checking struct field tags

649b899

Signed-off-by: Alex Boten <[email protected]>

tidy

e462064

Signed-off-by: Alex Boten <[email protected]>

fix lint

40d3021

Signed-off-by: Alex Boten <[email protected]>

codeboten force-pushed the codeboten/update-config-dependency branch from c526989 to 40d3021 Compare January 17, 2025 21:17

codeboten requested review from dmathieu, evan-bradley and andrzej-stencel as code owners January 17, 2025 21:17

revert change to remove toolchain in otelcorecol go.mod

a8cdf12

Signed-off-by: Alex Boten <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update config dependency #11611

update config dependency #11611

codeboten commented Nov 5, 2024 •

edited

Loading

mjnowen commented Nov 12, 2024

github-actions bot commented Nov 27, 2024

mjnowen commented Nov 27, 2024 •

edited

Loading

codecov bot commented Dec 9, 2024 •

edited

Loading

codeboten commented Dec 10, 2024

dmitryax commented Dec 10, 2024

dmitryax Dec 10, 2024

mx-psi Dec 10, 2024

jpkrohling left a comment •

edited

Loading

jpkrohling Dec 10, 2024

mx-psi Dec 10, 2024

mx-psi left a comment

mx-psi Dec 10, 2024

mx-psi Dec 10, 2024

github-actions bot commented Jan 1, 2025

update config dependency #11611

Are you sure you want to change the base?

update config dependency #11611

Conversation

codeboten commented Nov 5, 2024 • edited Loading

mjnowen commented Nov 12, 2024

github-actions bot commented Nov 27, 2024

mjnowen commented Nov 27, 2024 • edited Loading

codecov bot commented Dec 9, 2024 • edited Loading

Codecov Report

codeboten commented Dec 10, 2024

tl;dr there's a backwards incompatible change in how headers are passed in and I wanted to support users transitioning

dmitryax commented Dec 10, 2024

dmitryax Dec 10, 2024

Choose a reason for hiding this comment

mx-psi Dec 10, 2024

Choose a reason for hiding this comment

jpkrohling left a comment • edited Loading

Choose a reason for hiding this comment

jpkrohling Dec 10, 2024

Choose a reason for hiding this comment

mx-psi Dec 10, 2024

Choose a reason for hiding this comment

mx-psi left a comment

Choose a reason for hiding this comment

mx-psi Dec 10, 2024

Choose a reason for hiding this comment

mx-psi Dec 10, 2024

Choose a reason for hiding this comment

github-actions bot commented Jan 1, 2025

codeboten commented Nov 5, 2024 •

edited

Loading

mjnowen commented Nov 27, 2024 •

edited

Loading

codecov bot commented Dec 9, 2024 •

edited

Loading

jpkrohling left a comment •

edited

Loading