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

[processor/k8sattributes] Add support for profiles signal #35999

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

haoqixu
Copy link
Member

@haoqixu haoqixu commented Oct 25, 2024

Description

Add support for profiles signal to k8sattributesprocessor.

Link to tracking issue

Fixes #35983

Testing

  • factory_test.go
  • processor_test.go

@github-actions github-actions bot added the processor/k8sattributes k8s Attributes processor label Oct 25, 2024
@haoqixu haoqixu force-pushed the f-profiles-k8sattributesprocessor branch from f4a8b82 to 70404bc Compare October 25, 2024 16:58
@haoqixu haoqixu marked this pull request as ready for review October 25, 2024 17:28
@haoqixu haoqixu requested a review from a team as a code owner October 25, 2024 17:28
@haoqixu haoqixu force-pushed the f-profiles-k8sattributesprocessor branch from 0b7efee to 00ff5b5 Compare October 29, 2024 14:55
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

This LGTM, but I would like to do the update-otel separately, so all core modules point to the same commit

@haoqixu
Copy link
Member Author

haoqixu commented Oct 30, 2024

This LGTM, but I would like to do the update-otel separately, so all core modules point to the same commit

The go.opentelemetry.io/collector/receiver/receivertest package has been moved into a separate module on the main branch and does not have a release tag yet. Running multimod will always fail on components depending on receivertest. 🤔

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.

Can we cover profiles in the e2e testing as well? See https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.112.0/processor/k8sattributesprocessor/e2e_test.go. Either tuning the existing tests or introducing a new one.

@haoqixu
Copy link
Member Author

haoqixu commented Nov 1, 2024

Can we cover profiles in the e2e testing as well? See https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.112.0/processor/k8sattributesprocessor/e2e_test.go. Either tuning the existing tests or introducing the a new one.

I have updated the e2e test. However, the telemetrygen CLI doesn't support profile signals yet. I will submit another PR to add support for profile signals to telemetrygen and then update this PR.

@haoqixu
Copy link
Member Author

haoqixu commented Nov 1, 2024

I will submit another PR to add support for profile signals to telemetrygen and then update this PR.

I have opened an issue (#36127) to track it and skip the tests with added TODOs. I would like to submit another PR to enable the profiles test and remove all TODOs from the tests once telemetrygen is ready for profiles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connector/datadog exporter/datadog Datadog components processor/k8sattributes k8s Attributes processor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[processor/k8sattributes] Add support for profiles signal
5 participants