Skip to content

[receiver/k8seventsreceiver] Add more types to severity map #43402

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

[receiver/k8seventsreceiver] Add more types to severity map #43402

wants to merge 24 commits into from
+85 −15

Conversation

gaddas3
Copy link

@gaddas3 gaddas3 commented Oct 9, 2025 •
edited
Loading

Description

Add more types to severity map.
k8s event api suggest more types can be added
https://github.com/kubernetes/api/blob/release-1.34/events/v1/types_swagger_doc_generated.go#L42
To support additional types, set the event type as is and if it is a known type set the SeverityNumber other wise set SeverityNumberUnspecified

Link to tracking issue

Fixes #43401

Testing

ubuntu@W117ZS8FS3:/mnt/c/Users/gaddas3/workspace/opentelemetry-collector-contrib/receiver/k8seventsreceiver$ go test
PASS
ok      github.com/open-telemetry/opentelemetry-collector-contrib/receiver/k8seventsreceiver    0.223s

Documentation

Created change log with the following subtext:
k8seventsreceiver allows event types Error and Critical in addition to current Normal and Warning event types.

@gaddas3 gaddas3 requested review from a team, ChrsMark, TylerHelmuth and dmitryax as code owners October 9, 2025 21:05
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Oct 9, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@github-actions github-actions bot added the first-time contributor PRs made by new contributors label Oct 9, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 9, 2025

Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib.

Important reminders:

A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better!

bogdan-st
bogdan-st reviewed Oct 10, 2025
View reviewed changes
Copy link
Contributor

@bogdan-st bogdan-st left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!
Please read the contribution guide and follow the changelog instructions, there is a test that will fail.
Also, a couple of observations and questions

receiver/k8seventsreceiver/k8s_event_to_logdata.go
var severityMap = map[string]plog.SeverityNumber{
"normal": plog.SeverityNumberInfo,
"warning": plog.SeverityNumberWarn,
"error": plog.SeverityNumberError,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason you're not just adding the new types here? I do agree that this implementation is preserving some more info but I'm not sure it s needed.

Copy link
Author

Choose a reason for hiding this comment

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

Please elaborate on this. Are referring to adding other types https://github.com/open-telemetry/opentelemetry-collector/blob/main/pdata/plog/severity_number.go#L14-L38

Copy link
Contributor

Choose a reason for hiding this comment

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

It was a bad use of words on my part. I meant just adding them there, without the rest of the logic (which is the state of the PR now)

@github-actions GitHub Actions
Copy link
Contributor

Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib.

Important reminders:

A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better!

bogdan-st
bogdan-st reviewed Oct 15, 2025
View reviewed changes
Copy link
Contributor

@bogdan-st bogdan-st left a comment

Choose a reason for hiding this comment

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

just a small indentation issue
also please look into filling the changelog and change the issue title to include the component name.
lgtm otherwise

@gaddas3 gaddas3 changed the title Add more types to severity map [receiver/k8seventsreceiver] Add more types to severity map Oct 15, 2025
@gaddas3 gaddas3 requested a review from bogdan-st October 15, 2025 15:16
@bogdan-st
Copy link
Contributor

Please fill all the fields of the changelog file

@gaddas3
Copy link
Author

gaddas3 commented Oct 16, 2025

Please fill all the fields of the changelog file
Updated. Thanks for reviewing. Is it possible to run workflows before approval?

@gaddas3
Copy link
Author

gaddas3 commented Oct 16, 2025

I am not able to build locally. Getting connection reset errors frequently. Would it be possible to run tests via github actions?

make otelcontribcol

go: github.com/open-telemetry/opentelemetry-collector-contrib/cmd/otelcontribcol imports
        github.com/open-telemetry/opentelemetry-collector-contrib/confmap/provider/s3provider imports
        github.com/aws/aws-sdk-go-v2/service/s3: github.com/aws/aws-sdk-go-v2/service/s3@v1.88.4: verifying module: github.com/aws/aws-sdk-go-v2/service/s3@v1.88.4: Get "https://sum.golang.org/lookup/github.com/aws/aws-sdk-go-v2/service/s3@v1.88.4": read tcp 172.28.0.157:51594->142.251.116.141:443: read: connection reset by peer
go: github.com/open-telemetry/opentelemetry-collector-contrib/cmd/otelcontribcol imports
        github.com/open-telemetry/opentelemetry-collector-contrib/confmap/provider/secretsmanagerprovider imports
        github.com/aws/aws-sdk-go-v2/service/secretsmanager: github.com/aws/aws-sdk-go-v2/service/secretsmanager@v1.39.6: Get "https://proxy.golang.org/github.com/aws/aws-sdk-go-v2/service/secretsmanager/@v/v1.39.6.zip": read tcp 172.28.0.157:46412->142.250.113.141:443: read: connection reset by peer

@gaddas3
Copy link
Author

gaddas3 commented Oct 17, 2025

Please suggest next steps, would like to see this merged, so that we don't have to fork this repo for our customers. Thanks

@gaddas3 gaddas3 mentioned this pull request Oct 17, 2025
Open
ChrsMark
ChrsMark reviewed Oct 20, 2025
View reviewed changes
Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

I'm fine with this addition. Could you add a unit test for these added levels?

@gaddas3
Copy link
Author

gaddas3 commented Oct 20, 2025

I'm fine with this addition. Could you add a unit test for these added levels?

Please review again, updated the unit tests.

@gaddas3 gaddas3 requested a review from ChrsMark October 20, 2025 17:09
ChrsMark
ChrsMark approved these changes Oct 21, 2025
View reviewed changes
Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

The change looks good to me, however since these types are not officially supported by K8s I would like other code-owners to also approve this. My thought is that by mapping already error and critical is not that arbitrary even if that's not what K8s officially exposes.

@gaddas3
Copy link
Author

gaddas3 commented Oct 21, 2025

Please fill all the fields of the changelog file

@bogdan-st please review this again. Thanks

@gaddas3
Copy link
Author

gaddas3 commented Oct 21, 2025

@ChrsMark could you please approve GitHub actions workflow, would like to see all the required statuses are good. Thanks

@gaddas3
Copy link
Author

gaddas3 commented Oct 21, 2025

@dmitryax please see if you can review this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChrsMark ChrsMark ChrsMark approved these changes

@dmitryax dmitryax Awaiting requested review from dmitryax dmitryax is a code owner

@TylerHelmuth TylerHelmuth Awaiting requested review from TylerHelmuth TylerHelmuth is a code owner

@bogdan-st bogdan-st Awaiting requested review from bogdan-st

Assignees

@songy23 songy23

Labels

first-time contributor PRs made by new contributors receiver/k8sevents waiting-for-code-owners

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

k8seventreceiver should allow event types other than normal and warning

5 participants

@gaddas3 @bogdan-st @ChrsMark @atoulme @songy23