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

[receiver/awsfirehosereceiver] make otlp_v1 a valid record type #35750

Merged
merged 7 commits into from
Oct 25, 2024

Conversation

kaiyan-sheng
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng commented Oct 11, 2024

Description

This PR is to add otlp_v1 into the list of valid record_type input.
Otherwise when record_type sets to otlp_v1, we get error:

2024-10-23T16:53:09.823Z	info	service@v0.112.0/service.go:135	Setting up own telemetry...
2024-10-23T16:53:09.824Z	info	telemetry/metrics.go:70	Serving metrics	{"address": "localhost:8888", "metrics level": "Normal"}
Error: failed to build pipelines: failed to create "awsfirehose" receiver for data type "metrics": unrecognized record type

@gavincabbage
Copy link
Contributor

I was wondering if this would be broken when I saw the release notes yesterday. To add log support in #35077 I had to mess with the record type handling, which probably wasn't considered during the concurrent development of #34982 merged the day after.

@andrzej-stencel andrzej-stencel merged commit df5ba72 into open-telemetry:main Oct 25, 2024
158 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 25, 2024
@kaiyan-sheng kaiyan-sheng deleted the add_otlp branch October 25, 2024 14:06
mx-psi pushed a commit that referenced this pull request Nov 4, 2024
…36125)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

After able to test E2E with
#35750,
I realized `otlp_v1` is still treated as an invalid record type because
it's missing from the unmarshalers map.

This PR is to add it into the map, enhance the error message output to
show the `record_type` and add a test yaml file.
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
…-telemetry#35750)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This PR is to add `otlp_v1` into the list of valid `record_type` input.
Otherwise when `record_type` sets to `otlp_v1`, we get error:
```
2024-10-23T16:53:09.823Z	info	service@v0.112.0/service.go:135	Setting up own telemetry...
2024-10-23T16:53:09.824Z	info	telemetry/metrics.go:70	Serving metrics	{"address": "localhost:8888", "metrics level": "Normal"}
Error: failed to build pipelines: failed to create "awsfirehose" receiver for data type "metrics": unrecognized record type
```
<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->

---------

Co-authored-by: Matthew Wear <matthew.wear@gmail.com>
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
…pen-telemetry#36125)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

After able to test E2E with
open-telemetry#35750,
I realized `otlp_v1` is still treated as an invalid record type because
it's missing from the unmarshalers map.

This PR is to add it into the map, enhance the error message output to
show the `record_type` and add a test yaml file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants