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

[doc][filter processor] #18865

Closed
edwintye opened this issue Feb 21, 2023 · 6 comments
Closed

[doc][filter processor] #18865

edwintye opened this issue Feb 21, 2023 · 6 comments
Assignees
Labels
bug Something isn't working priority:p2 Medium processor/filter Filter processor

Comments

@edwintye
Copy link
Contributor

edwintye commented Feb 21, 2023

Component(s)

processor/filter

What happened?

Description

We created a config that followed the examples on the README.md and it started failing on release v0.71.0. The issue seems to be due to the upstream change open-telemetry/opentelemetry-collector#6876 which means that keys are now case sensitive. The examples on the README.md however has (multiple) blocks like

processors:
  filter:
    spans:
      include:
        match_type: strict
        services:
          - app_3
      exclude:
        match_type: regexp
        services:
          - app_1
          - app_2
        span_names:
          - hello_world
          - hello/world
        attributes:
          - Key: container.name
            Value: (app_container_1|app_container_2)
        libraries:
          - Name: opentelemetry
            Version: 0.0-beta
        resources:
          - Key: container.host
            Value: (localhost|127.0.0.1)

where Key and Value should be key and value instead. The use of Key/Value also differs from the test configs under /testdata.

Steps to Reproduce

We don't need to use a full otel config to test, we can simply adjust the test data to have the incorrect key, i.e. in processor/filterprocessor/testdata/config_traces.yaml change key and value to Key and Value then run the test

[$processor/filterprocessor] go test -v -run TestLoadingSpans

which will fail. Running the same test on tag v0.70.0 passes.

Expected Result

The tests passes - or maybe fail depending on your POV but I feel like this is an unintended breaking change.

Actual Result

The tests fails with output

    config_test.go:610:
        	Error Trace:	/Users/edwintye/github/opentelemetry-collector-contrib/processor/filterprocessor/config_test.go:610
        	Error:      	Received unexpected error:
        	            	1 error(s) decoding:

        	            	* 'spans.include.attributes[0]' has invalid keys: Key, Value

Collector version

v0.71.0

Environment information

N/A

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

I don't know how wild spread this issue is and how many people copy the examples from the README.md. I have discovered in the filter processor because it broke our deployment but this issue may apply everywhere.

@edwintye edwintye added bug Something isn't working needs triage New item requiring triage labels Feb 21, 2023
@github-actions github-actions bot added the processor/filter Filter processor label Feb 21, 2023
@github-actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@TylerHelmuth TylerHelmuth added priority:p2 Medium and removed needs triage New item requiring triage labels Feb 22, 2023
@TylerHelmuth
Copy link
Member

@edwintye thanks for pointing this out. Its true that the linked change had some side effects. Are you planning to submit a PR for this issue? If so, I'll assign it to you.

@edwintye
Copy link
Contributor Author

@TylerHelmuth Thanks for response. Is it just to improve the readme? If so I can do that. I can write a couple of test too although I am not sure how much benefit they will provide now that the change already happened :)

@TylerHelmuth
Copy link
Member

@edwintye Updating the README is what we need.

@edwintye
Copy link
Contributor Author

yea sure, I will find the time in the next couple of days.

@edwintye
Copy link
Contributor Author

Closing issue with PR merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p2 Medium processor/filter Filter processor
Projects
None yet
Development

No branches or pull requests

2 participants