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 81.21547% with 68 lines in your changes missing coverage. Please review.

Project coverage is 91.68%. Comparing base (808e4a8) to head (7220af9).

Files with missing lines	Patch %	Lines
service/service.go	72.38%	34 Missing and 3 partials ⚠️
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 ⚠️
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.82%   91.68%   -0.15%     
==========================================
  Files         463      464       +1     
  Lines       24776    24776              
==========================================
- Hits        22750    22715      -35     
- Misses       1644     1675      +31     
- 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.

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 · 2025-01-22T20:24:53Z

service/service.go

+			InstrumentName: ptr("otelcol_processor_batch_batch_send_size_bytes"),
+		}))
+	}
+	return views


@mx-psi ended up moving the logic to append views in here, as the views are needed to instantiate the SDK, PTAL

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

codeboten · 2025-01-22T23:37:00Z

Attempting to fix the integration test that's failing in contrib: open-telemetry/opentelemetry-collector-contrib#37425

Unclear to me whether this change is making that test flakier or not as this issue exists: open-telemetry/opentelemetry-collector-contrib#34836

codeboten · 2025-01-23T06:28:29Z

Another attempt at fixing the contrib test open-telemetry/opentelemetry-collector-contrib#37431

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 added 21 commits January 22, 2025 10:48

set meter provider config in SDK initialization

284b4fa

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

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

a0ba9ff

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

fix attributes test

50c047b

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

move code to disable high cardinality to use views in service

f5ba523

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

add changelog

ab5ace1

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

ensure metric level none disables metric readers

a988588

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

ensure the sdk is shutdown on error

c7b6ce6

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

fix service_test

6f41b4a

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

add migration from v0.2.0 to v0.3.0 support

28afb8d

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

bff96d6

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

add v0.3.0 tests

ccc4470

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

update imports

c7d1ed6

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

fix additional imports

9dabf0f

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

fix package statements

3936158

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

update test to reflect error message

539948d

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

ignore AdditionalProperties when checking struct field tags

724fb72

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

tidy

d186a44

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

fix lint

92edfd8

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

revert change to remove toolchain in otelcorecol go.mod

ab33a92

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

crosslink

6982455

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

tidy

f36069b

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

codeboten force-pushed the codeboten/update-config-dependency branch from 9791d42 to 52c4f85 Compare January 22, 2025 20:18

codeboten commented Jan 22, 2025

View reviewed changes

codeboten force-pushed the codeboten/update-config-dependency branch from 52c4f85 to d314b4d Compare January 22, 2025 20:26

fix test

d742afd

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

codeboten force-pushed the codeboten/update-config-dependency branch from d314b4d to d742afd Compare January 22, 2025 20:28

more lint

c5ca4ea

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

Merge branch 'main' into codeboten/update-config-dependency

7220af9

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

codeboten Jan 22, 2025

codeboten commented Jan 22, 2025

codeboten commented Jan 23, 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 Jan 22, 2025

Choose a reason for hiding this comment

codeboten commented Jan 22, 2025

codeboten commented Jan 23, 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