Enhance config validation by Instantiating `service` during dryrun. #12488

sudiptob2 · 2025-02-25T22:03:26Z

Description

As mentioned in #8721 (comment), we can surface configuration errors by instantiating the service during a dry run.

Expanding on this idea, this PR enhances configuration validation. The validate command will now capture all validation errors that would prevent the collector from starting.

Link to the tracking issue

Fixes #8721

Testing

Added unit tests. Also, a few scenarios and example output are given below. I will add more unit tests if necessary.

1. Improper Connector Use

invalid_config.yaml

receivers:
  otlp:
    protocols:
      grpc:
exporters:
  debug:
connectors:
  forward:
service:
  pipelines:
    logs/in:
      receivers: [otlp]
      processors: []
      exporters: [forward]
    logs/in2:
      receivers: [ otlp ]
      processors: [ ]
      exporters: [ forward ]
    logs/out:
      receivers: [otlp]
      processors: []
      exporters: [debug]
    traces:
      receivers: [ otlp ]
      processors: [ ]
      exporters: [ forward ]
    metrics:
      receivers: [ forward ]
      processors: [ ]
      exporters: [ debug ]

Before: No error detected

Proposed Change:

Error: failed to build pipelines: connector "forward" used as exporter in [logs/in2 logs/in] pipeline but not used in any supported receiver pipeline
2025/02/25 16:58:09 collector server run finished with error: failed to build pipelines: connector "forward" used as exporter in [logs/in2 logs/in] pipeline but not used in any supported receiver pipeline

2. Unused Connector

invalid_config.yaml

receivers:
  otlp:
    protocols:
      grpc:
      http:
exporters:
  debug:
connectors:
  forward:
service:
  pipelines:
    logs/out:
      receivers: [forward]
      processors: []
      exporters: [debug]

Before: No error detected

Proposed Change:

Error: failed to build pipelines: connector "forward" used as receiver in [logs/out] pipeline but not used in any supported exporter pipeline
2025/02/26 10:40:53 collector server run finished with error: failed to build pipelines: connector "forward" used as receiver in [logs/out] pipeline but not used in any supported exporter pipeline

2. Cycle in the Pipeline

invalid_config.yaml

receivers:
  otlp:
    protocols:
      grpc:

exporters:
  debug:

connectors:
  forward:

service:
  pipelines:
    traces/in:
      receivers: [forward]
      processors: [ ]
      exporters: [forward]
    traces/out:
      receivers: [forward]
      processors: [ ]
      exporters: [forward]

Before: No error detected

Proposed Change:

Error: failed to build pipelines: cycle detected: connector "forward" (traces to traces) -> connector "forward" (traces to traces)
2025/02/26 11:12:53 collector server run finished with error: failed to build pipelines: cycle detected: connector "forward" (traces to traces) -> connector "forward" (traces to traces)

Note

I removed the following log as it seemed redundant to me and it is also surfaced during validation. Let me know if we absolutely need to keep it

opentelemetry-collector/service/service.go

Line 193 in 5812712

logger.Info("Setting up own telemetry...")

codecov · 2025-02-25T22:12:32Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.60%. Comparing base (ee0f0ae) to head (e53a1f7).
Report is 72 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12488      +/-   ##
==========================================
+ Coverage   91.57%   91.60%   +0.03%     
==========================================
  Files         483      483              
  Lines       26383    26386       +3     
==========================================
+ Hits        24160    24172      +12     
+ Misses       1762     1756       -6     
+ Partials      461      458       -3

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sudiptob2 · 2025-02-26T04:28:40Z

Some tests are failing, which are not related to this PR. Any guidance would be helpful.

mx-psi

What are the errors that were not reported before that are reported after this change? Can we instead statically validate those?

mx-psi · 2025-02-26T11:52:59Z

Feel free to ignore the errors, they are not related to this PR

sudiptob2 · 2025-02-26T17:36:06Z

@mx-psi Thanks for the quick response!

What are the errors that were not reported before that are reported after this change? Can we instead statically validate those?

This approach should catch all errors in the pipeline configuration. I am not aware of it all, but here are a couple of examples.

Especially for $\textcolor{red}{\textsf{cycle detection,}}$ we need to initialize the graph. Statically verifying that might not be feasible.

1. Improper Connector Use

invalid_config.yaml

receivers:
  otlp:
    protocols:
      grpc:
exporters:
  debug:
connectors:
  forward:
service:
  pipelines:
    logs/in:
      receivers: [otlp]
      processors: []
      exporters: [forward]
    logs/in2:
      receivers: [ otlp ]
      processors: [ ]
      exporters: [ forward ]
    logs/out:
      receivers: [otlp]
      processors: []
      exporters: [debug]
    traces:
      receivers: [ otlp ]
      processors: [ ]
      exporters: [ forward ]
    metrics:
      receivers: [ forward ]
      processors: [ ]
      exporters: [ debug ]

Before: No error detected

Proposed Change:

Error: failed to build pipelines: connector "forward" used as exporter in [logs/in2 logs/in] pipeline but not used in any supported receiver pipeline
2025/02/25 16:58:09 collector server run finished with error: failed to build pipelines: connector "forward" used as exporter in [logs/in2 logs/in] pipeline but not used in any supported receiver pipeline

2. Unused Connector

invalid_config.yaml

receivers:
  otlp:
    protocols:
      grpc:
      http:
exporters:
  debug:
connectors:
  forward:
service:
  pipelines:
    logs/out:
      receivers: [forward]
      processors: []
      exporters: [debug]

Before: No error detected

Proposed Change:

Error: failed to build pipelines: connector "forward" used as receiver in [logs/out] pipeline but not used in any supported exporter pipeline
2025/02/26 10:40:53 collector server run finished with error: failed to build pipelines: connector "forward" used as receiver in [logs/out] pipeline but not used in any supported exporter pipeline

2. Cycle in the Pipeline

invalid_config.yaml

receivers:
  otlp:
    protocols:
      grpc:

exporters:
  debug:

connectors:
  forward:

service:
  pipelines:
    traces/in:
      receivers: [forward]
      processors: [ ]
      exporters: [forward]
    traces/out:
      receivers: [forward]
      processors: [ ]
      exporters: [forward]

Before: No error detected

Proposed Change:

Error: failed to build pipelines: cycle detected: connector "forward" (traces to traces) -> connector "forward" (traces to traces)
2025/02/26 11:12:53 collector server run finished with error: failed to build pipelines: cycle detected: connector "forward" (traces to traces) -> connector "forward" (traces to traces)

otelcol/collector.go

bogdandrutu

I would prefer the DryRun to not have to start the collector, what do we miss that we need to do this?

sudiptob2 · 2025-02-28T23:25:19Z

I would prefer the DryRun to not have to start the collector, what do we miss that we need to do this?

These are the error that current validate command can not detect

1. Improper Connector Use

invalid_config.yaml

receivers:
  otlp:
    protocols:
      grpc:
exporters:
  debug:
connectors:
  forward:
service:
  pipelines:
    logs/in:
      receivers: [otlp]
      processors: []
      exporters: [forward]
    logs/in2:
      receivers: [ otlp ]
      processors: [ ]
      exporters: [ forward ]
    logs/out:
      receivers: [otlp]
      processors: []
      exporters: [debug]
    traces:
      receivers: [ otlp ]
      processors: [ ]
      exporters: [ forward ]
    metrics:
      receivers: [ forward ]
      processors: [ ]
      exporters: [ debug ]

Before: No error detected

Proposed Change:

Error: failed to build pipelines: connector "forward" used as exporter in [logs/in2 logs/in] pipeline but not used in any supported receiver pipeline
2025/02/25 16:58:09 collector server run finished with error: failed to build pipelines: connector "forward" used as exporter in [logs/in2 logs/in] pipeline but not used in any supported receiver pipeline

2. Unused Connector

invalid_config.yaml

receivers:
  otlp:
    protocols:
      grpc:
      http:
exporters:
  debug:
connectors:
  forward:
service:
  pipelines:
    logs/out:
      receivers: [forward]
      processors: []
      exporters: [debug]

Before: No error detected

Proposed Change:

Error: failed to build pipelines: connector "forward" used as receiver in [logs/out] pipeline but not used in any supported exporter pipeline
2025/02/26 10:40:53 collector server run finished with error: failed to build pipelines: connector "forward" used as receiver in [logs/out] pipeline but not used in any supported exporter pipeline

2. Cycle in the Pipeline

invalid_config.yaml

receivers:
  otlp:
    protocols:
      grpc:

exporters:
  debug:

connectors:
  forward:

service:
  pipelines:
    traces/in:
      receivers: [forward]
      processors: [ ]
      exporters: [forward]
    traces/out:
      receivers: [forward]
      processors: [ ]
      exporters: [forward]

Before: No error detected

Proposed Change:

Error: failed to build pipelines: cycle detected: connector "forward" (traces to traces) -> connector "forward" (traces to traces)
2025/02/26 11:12:53 collector server run finished with error: failed to build pipelines: cycle detected: connector "forward" (traces to traces) -> connector "forward" (traces to traces)

github-actions · 2025-03-15T03:17:47Z

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

sudiptob2 · 2025-03-22T13:40:15Z

@bogdandrutu PR #12681, implements validation without instantiating the service, please take a look.

github-actions · 2025-04-08T03:22:48Z

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

#### Description As described in [#8721](#8721), the current `validate` command cannot capture improper connector usage errors during a dry run. One proposed solution _(as mentioned in [this comment](#8721 (comment) is to surface configuration errors by instantiating the service during the dry run. However, reviewers preferred statically verifying the errors over instantiating the service (see the [PR #12488 comments](#12488)). The validation logic for connectors is located in the [`service/internal/graph`](https://github.com/open-telemetry/opentelemetry-collector/blob/1daa315455d02bac90185027878d858ba08a0f07/service/internal/graph) package. In the discussion of [PR #12681](#12681), it was concluded that instantiating the graph is the better approach (see [this comment](#12681 (comment))). Finally, this PR uses the `graph.Build()` method to surface configuration errors during the dry run.  #### Link to tracking issue Fixes #8721 #12535  #### Testing  #### Documentation  --------- Signed-off-by: sudipto baral <[email protected]>

bogdandrutu

Please rebase

bogdandrutu · 2025-04-10T20:17:47Z

otelcol/unmarshal_dry_run_test.go

 			mockMap: map[string]string{
 				"number": "123",
 			},
+			expectErr: true,


Please add a version without error.

bogdandrutu · 2025-04-10T20:18:22Z

otelcol/collector.go

-	if err = conf.Marshal(cfg); err != nil {
-		return fmt.Errorf("could not marshal configuration: %w", err)
-	}
+	_ = conf.Marshal(cfg)


why skip the error?

sudiptob2 · 2025-04-10T20:23:17Z

We can close this one as #12712 was the final PR which is now merged. @bogdandrutu

sudiptob2 force-pushed the fix/8721/config-validation branch 4 times, most recently from 89f448f to 754e9a7 Compare February 26, 2025 03:56

sudiptob2 marked this pull request as ready for review February 26, 2025 04:18

sudiptob2 requested a review from a team as a code owner February 26, 2025 04:18

sudiptob2 requested a review from mx-psi February 26, 2025 04:18

mx-psi reviewed Feb 26, 2025

View reviewed changes

sudiptob2 force-pushed the fix/8721/config-validation branch 2 times, most recently from 16f3e68 to bce7fac Compare February 27, 2025 20:10

sudiptob2 requested a review from mx-psi February 28, 2025 01:40

bogdandrutu reviewed Feb 28, 2025

View reviewed changes

otelcol/collector.go Show resolved Hide resolved

bogdandrutu requested changes Feb 28, 2025

View reviewed changes

github-actions bot added the Stale label Mar 15, 2025

sudiptob2 mentioned this pull request Mar 20, 2025

Static validation of improper connector usage in config files during dry-run #12681

Closed

sudiptob2 requested a review from bogdandrutu March 22, 2025 13:54

sudiptob2 added 4 commits March 22, 2025 09:55

Enhance config validation by Instantiating service during dryrun.

7228ae0

Add test for coverage.

eaf0e47

Error is always nil in this method

e1470dc

Add test for cyclic connector use.

e53a1f7

sudiptob2 force-pushed the fix/8721/config-validation branch from bce7fac to e53a1f7 Compare March 22, 2025 13:55

sudiptob2 mentioned this pull request Mar 24, 2025

Statically validate connector config during dry-run. #12712

Merged

github-actions bot removed the Stale label Mar 25, 2025

github-actions bot added the Stale label Apr 8, 2025

bogdandrutu approved these changes Apr 10, 2025

View reviewed changes

sudiptob2 closed this Apr 10, 2025

bogdandrutu reopened this Apr 10, 2025

bogdandrutu closed this Apr 10, 2025

Enhance config validation by Instantiating service during dryrun. #12488

Enhance config validation by Instantiating service during dryrun. #12488

Uh oh!

Conversation

sudiptob2 commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Link to the tracking issue

Testing

Note

Uh oh!

codecov bot commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sudiptob2 commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mx-psi left a comment

Choose a reason for hiding this comment

Uh oh!

mx-psi commented Feb 26, 2025

Uh oh!

sudiptob2 commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bogdandrutu left a comment

Choose a reason for hiding this comment

Uh oh!

sudiptob2 commented Feb 28, 2025

Uh oh!

github-actions bot commented Mar 15, 2025

Uh oh!

sudiptob2 commented Mar 22, 2025

Uh oh!

github-actions bot commented Apr 8, 2025

Uh oh!

bogdandrutu left a comment

Choose a reason for hiding this comment

Uh oh!

bogdandrutu Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

bogdandrutu Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

sudiptob2 commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Enhance config validation by Instantiating `service` during dryrun. #12488

Enhance config validation by Instantiating `service` during dryrun. #12488

sudiptob2 commented Feb 25, 2025 •

edited

Loading

codecov bot commented Feb 25, 2025 •

edited

Loading

sudiptob2 commented Feb 26, 2025 •

edited

Loading

sudiptob2 commented Feb 26, 2025 •

edited

Loading

sudiptob2 commented Apr 10, 2025 •

edited

Loading