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

awsfirehosereceiver: ignore CONTROL_MESSAGE log records #38445

Merged
merged 3 commits into from
Mar 12, 2025

Conversation

axw
Copy link
Contributor

@axw axw commented Mar 7, 2025

Description

Fix the CloudWatch logs unmarshaler so it ignores CONTROL_MESSAGE log records, rather than returning an error. As mentioned at https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/SubscriptionFilters.html, CONTROL_MESSAGE records are produced for health-checking the destination.

Link to tracking issue

Relates to #38433

(Not sure yet if it fixes it.)

Testing

  • Added a unit test case.
  • Created a Firehose delivery stream & CloudWatch subscription filter, pointing at the collector

Documentation

N/A

Fix the logs unmarshaller so it ignores
CONTROL_MESSAGE CloudWatch log records,
rather than returning an error.
@axw axw requested a review from edmocosta March 10, 2025 01:01
@edmocosta edmocosta added ready to merge Code review completed; ready to merge by maintainers and removed waiting-for-code-owners labels Mar 11, 2025
atoulme pushed a commit that referenced this pull request Mar 11, 2025
#### Description

I would like to become a codeowner of the awsfirehosereceiver. Elastic
(where I work) intends to use this receiver heavily, and I would like
to:
 - help spread out the maintenance load
- ensure we have a path to progressing beyond the current Alpha
stability

Assuming
open-telemetry/opentelemetry-collector#11864
goes through, it will be necessary to have multiple codeowners to
progress to Beta and preferable to have codeowners from multiple
employers, which is the case for @Aneurysm9 (AWS) and me (Elastic).

Relevant changes:
-
#37111
(superseded by #37361)
-
#37262
-
#37361
-
#38388
(will extract from awsfirehosereceiver)
-
#38445

#### Link to tracking issue

N/A

#### Testing

N/A

#### Documentation

N/A
@atoulme atoulme closed this Mar 11, 2025
@atoulme atoulme reopened this Mar 11, 2025
Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need rebase / merge main

@songy23 songy23 removed the ready to merge Code review completed; ready to merge by maintainers label Mar 11, 2025
@axw
Copy link
Contributor Author

axw commented Mar 12, 2025

/label "ready to merge"

@atoulme atoulme merged commit 7c543b8 into open-telemetry:main Mar 12, 2025
171 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 12, 2025
@axw axw deleted the firehose-control-messages branch March 12, 2025 02:17
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.

6 participants