Give NoOp create settings a unique name #9637

dashpole · 2024-02-26T15:14:52Z

Description:

Long story, but i'm working on updating the prometheus dependency: open-telemetry/opentelemetry-collector-contrib#30934

As part of that update, we need to adapt to a change that makes the prometheus servers' self-observability metrics independent. See prometheus/prometheus#13507 and prometheus/prometheus#13610

One way to adapt to this change is by adding a label to each receivers' metrics to differentiate one Prometheus receiver from another. I've tried taking that approach in open-telemetry/opentelemetry-collector-contrib#30934, but the current issue is that the NoOp components all have the same name, which causes the self-observability metrics to collide.

I can work around this in the prometheus receiver's own tests, but I can't work around the issue in the generated_component_test.go tests, since those are generated.

This PR makes the ID returned by NewNopCreateSettings unique by giving it a unique name.

Link to tracking Issue:

Part of open-telemetry/opentelemetry-collector-contrib#30883

cc @Aneurysm9

codecov · 2024-02-28T16:53:12Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.98%. Comparing base (0c10f56) to head (6e875a5).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9637   +/-   ##
=======================================
  Coverage   90.98%   90.98%           
=======================================
  Files         349      349           
  Lines       18572    18572           
=======================================
  Hits        16897    16897           
  Misses       1348     1348           
  Partials      327      327

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

dashpole · 2024-02-28T17:35:10Z

This breaks a few tests in contrib, but it should be easy to fix those when we bump the dependency.

Aneurysm9

This generally looks good to me, but it should have a changelog entry as it may confound test expectations.

dashpole · 2024-02-29T02:38:25Z

Added a changelog.

dashpole · 2024-03-06T17:44:02Z

I'll need to update contrib first to fix the failing tests.

**Description:** This should fix failures in open-telemetry/opentelemetry-collector#9637 This unit test was relying on the ID from `exportertest.NewNopCreateSettings` always being the same. This change makes the test use the same create settings when storing the telemetry registry, and when creating the trace exporter to avoid that problem. **Link to tracking Issue:** Blocking open-telemetry/opentelemetry-collector#9637

@dmitryax

#31659) Follow-up to #31640 I didn't realize the contrib test stopped after the first failure, so there were a few more failures for open-telemetry/opentelemetry-collector#9637. I verified that this passes the contrib-tests check for the core repo locally, so this should be the rest of them. For the splunkhec exporter, the exporter actually uses `ID.String()`: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/4c5b5e934484ae3084565abbd3746a98e7f27721/exporter/splunkhecexporter/client.go#L71 The filelog receiver was assuming that `NewNopCreateSettings` returned the same ID, so I re-used the create settings in the test. **Link to tracking Issue:** Blocking open-telemetry/opentelemetry-collector#9637 @dmitryax @Aneurysm9 @codeboten

dashpole · 2024-03-11T13:38:19Z

seems like the changelog check is broken?

/usr/local/lib/node_modules/markdown-link-check/markdown-link-check:97
                    if (program.opts().ignore.some((ignorePath) => resolved.includes(ignorePath))) {

TypeError: Cannot read properties of undefined (reading 'some')
    at Command.<anonymous> (/usr/local/lib/node_modules/markdown-link-check/markdown-link-check:97:46)
    at Command.listener [as _actionHandler] (/usr/local/lib/node_modules/markdown-link-check/node_modules/commander/lib/command.js:482:17)
    at /usr/local/lib/node_modules/markdown-link-check/node_modules/commander/lib/command.js:1283:65
    at Command._chainOrCall (/usr/local/lib/node_modules/markdown-link-check/node_modules/commander/lib/command.js:1[17](https://github.com/open-telemetry/opentelemetry-collector/actions/runs/8233460550/job/22512960811?pr=9637#step:10:18)7:12)
    at Command._parseCommand (/usr/local/lib/node_modules/markdown-link-check/node_modules/commander/lib/command.js:1283:27)
    at Command.parse (/usr/local/lib/node_modules/markdown-link-check/node_modules/commander/lib/command.js:909:10)
    at getInputs (/usr/local/lib/node_modules/markdown-link-check/markdown-link-check:108:7)
    at main (/usr/local/lib/node_modules/markdown-link-check/markdown-link-check:252:[20](https://github.com/open-telemetry/opentelemetry-collector/actions/runs/8233460550/job/22512960811?pr=9637#step:10:21))

codeboten · 2024-03-12T19:17:07Z

Looks like it resolved :)

…lemetry#31640) **Description:** This should fix failures in open-telemetry/opentelemetry-collector#9637 This unit test was relying on the ID from `exportertest.NewNopCreateSettings` always being the same. This change makes the test use the same create settings when storing the telemetry registry, and when creating the trace exporter to avoid that problem. **Link to tracking Issue:** Blocking open-telemetry/opentelemetry-collector#9637

@dmitryax

open-telemetry#31659) Follow-up to open-telemetry#31640 I didn't realize the contrib test stopped after the first failure, so there were a few more failures for open-telemetry/opentelemetry-collector#9637. I verified that this passes the contrib-tests check for the core repo locally, so this should be the rest of them. For the splunkhec exporter, the exporter actually uses `ID.String()`: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/4c5b5e934484ae3084565abbd3746a98e7f27721/exporter/splunkhecexporter/client.go#L71 The filelog receiver was assuming that `NewNopCreateSettings` returned the same ID, so I re-used the create settings in the test. **Link to tracking Issue:** Blocking open-telemetry/opentelemetry-collector#9637 @dmitryax @Aneurysm9 @codeboten

…lemetry#31640) **Description:** This should fix failures in open-telemetry/opentelemetry-collector#9637 This unit test was relying on the ID from `exportertest.NewNopCreateSettings` always being the same. This change makes the test use the same create settings when storing the telemetry registry, and when creating the trace exporter to avoid that problem. **Link to tracking Issue:** Blocking open-telemetry/opentelemetry-collector#9637

@dmitryax

open-telemetry#31659) Follow-up to open-telemetry#31640 I didn't realize the contrib test stopped after the first failure, so there were a few more failures for open-telemetry/opentelemetry-collector#9637. I verified that this passes the contrib-tests check for the core repo locally, so this should be the rest of them. For the splunkhec exporter, the exporter actually uses `ID.String()`: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/4c5b5e934484ae3084565abbd3746a98e7f27721/exporter/splunkhecexporter/client.go#L71 The filelog receiver was assuming that `NewNopCreateSettings` returned the same ID, so I re-used the create settings in the test. **Link to tracking Issue:** Blocking open-telemetry/opentelemetry-collector#9637 @dmitryax @Aneurysm9 @codeboten

@Aneurysm9

Long story, but i'm working on updating the prometheus dependency: open-telemetry/opentelemetry-collector-contrib#30934 As part of that update, we need to adapt to a change that makes the prometheus servers' self-observability metrics independent. See prometheus/prometheus#13507 and prometheus/prometheus#13610 One way to adapt to this change is by adding a label to each receivers' metrics to differentiate one Prometheus receiver from another. I've tried taking that approach in open-telemetry/opentelemetry-collector-contrib#30934, but the current issue is that the NoOp components all have the same name, which causes the self-observability metrics to collide. I can work around this in the prometheus receiver's own tests, but I can't work around the issue in the `generated_component_test.go` tests, since those are generated. This PR makes the ID returned by `NewNopCreateSettings` unique by giving it a unique name. **Link to tracking Issue:** Part of open-telemetry/opentelemetry-collector-contrib#30883 cc @Aneurysm9 --------- Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>

dashpole requested review from a team and Aneurysm9 February 26, 2024 15:14

dashpole force-pushed the random_noop branch from 8c0cfe8 to 9ee4477 Compare February 28, 2024 16:46

Aneurysm9 reviewed Feb 29, 2024

View reviewed changes

Aneurysm9 approved these changes Feb 29, 2024

View reviewed changes

codeboten approved these changes Mar 6, 2024

View reviewed changes

dashpole changed the title ~~Give NoOp components a unique name~~ Give NoOp create settings a unique name Mar 6, 2024

dashpole marked this pull request as draft March 6, 2024 17:45

dashpole mentioned this pull request Mar 7, 2024

Adapt to randomly generated nop component names in unit test open-telemetry/opentelemetry-collector-contrib#31640

Merged

dashpole added 2 commits March 8, 2024 18:22

give noop components a unique name

9cd1f5f

changelog:

c5347e8

dashpole force-pushed the random_noop branch from 5bac891 to c5347e8 Compare March 8, 2024 18:22

dashpole mentioned this pull request Mar 8, 2024

Round 2: Adapt to randomly generated nop component names in unit tests open-telemetry/opentelemetry-collector-contrib#31659

Merged

Merge branch 'main' into random_noop

5a31262

dashpole marked this pull request as ready for review March 11, 2024 13:15

Merge branch 'main' into random_noop

6e875a5

codeboten merged commit dc411e3 into open-telemetry:main Mar 12, 2024
47 checks passed

github-actions bot added this to the next release milestone Mar 12, 2024

beorn7 mentioned this pull request Mar 13, 2024

scrape: Restore ability to create multiple scrape managers prometheus/prometheus#13610

Closed

dashpole mentioned this pull request Mar 14, 2024

Update collector core open-telemetry/opentelemetry-collector-contrib#31764

Closed

crobert-1 mentioned this pull request Mar 15, 2024

[chore] Update otel core libraries open-telemetry/opentelemetry-collector-contrib#31778

Merged

bogdandrutu mentioned this pull request Oct 13, 2024

Revert change to processortest.NopSettings name #11433

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give NoOp create settings a unique name #9637

Give NoOp create settings a unique name #9637

dashpole commented Feb 26, 2024

codecov bot commented Feb 28, 2024 •

edited

Loading

dashpole commented Feb 28, 2024

Aneurysm9 left a comment

dashpole commented Feb 29, 2024

dashpole commented Mar 6, 2024

dashpole commented Mar 11, 2024 •

edited

Loading

codeboten commented Mar 12, 2024

Give NoOp create settings a unique name #9637

Give NoOp create settings a unique name #9637

Conversation

dashpole commented Feb 26, 2024

codecov bot commented Feb 28, 2024 • edited Loading

Codecov Report

dashpole commented Feb 28, 2024

Aneurysm9 left a comment

Choose a reason for hiding this comment

dashpole commented Feb 29, 2024

dashpole commented Mar 6, 2024

dashpole commented Mar 11, 2024 • edited Loading

codeboten commented Mar 12, 2024

codecov bot commented Feb 28, 2024 •

edited

Loading

dashpole commented Mar 11, 2024 •

edited

Loading