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.

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

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

Merged
merged 10 commits into from
Nov 12, 2024
Merged

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

merged 10 commits into from
Nov 12, 2024

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
@haoqixu haoqixu requested a review from songy23 as a code owner October 29, 2024 16:56
@haoqixu haoqixu requested a review from mx-psi October 30, 2024 08:56
mx-psi
mx-psi approved these changes Oct 30, 2024
View reviewed changes
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. 🤔

ChrsMark
ChrsMark reviewed Oct 30, 2024
View reviewed changes
Copy link
Member

@ChrsMark ChrsMark left a comment •
edited
Loading

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 haoqixu force-pushed the f-profiles-k8sattributesprocessor branch from 78c7c27 to faef628 Compare November 1, 2024 06:47
@haoqixu haoqixu mentioned this pull request Nov 1, 2024
Open
@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.

@haoqixu haoqixu force-pushed the f-profiles-k8sattributesprocessor branch from af49672 to a0e57a0 Compare November 12, 2024 06:42
@haoqixu
Copy link
Member Author

haoqixu commented Nov 12, 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. 🤔

Hi, @mx-psi. I have rebased this PR to main, and the main branch has already updated core to v1.19.0/0.113.0.

TylerHelmuth
TylerHelmuth approved these changes Nov 12, 2024
View reviewed changes
Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

@fatsheep9146 @dmitryax please review.

@dmitryax dmitryax merged commit 2f2ba8c into open-telemetry:main Nov 12, 2024
158 checks passed
@github-actions github-actions bot added this to the next release milestone Nov 12, 2024
@dmitryax
Copy link
Member

Thanks @haoqixu!

sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
@haoqixu @sbylica-splunk
[processor/k8sattributes] Add support for profiles signal (open-telem…
8e21a90 
…etry#35999)

#### Description
Add support for profiles signal to `k8sattributesprocessor`.

#### Link to tracking issue
Fixes open-telemetry#35983

#### Testing
- factory_test.go
- processor_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrsMark ChrsMark ChrsMark left review comments

@TylerHelmuth TylerHelmuth TylerHelmuth approved these changes

@dmitryax dmitryax dmitryax approved these changes

@mx-psi mx-psi mx-psi approved these changes

@dmathieu dmathieu dmathieu approved these changes

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

@songy23 songy23 Awaiting requested review from songy23

@ankitpatel96 ankitpatel96 Awaiting requested review from ankitpatel96

@dineshg13 dineshg13 Awaiting requested review from dineshg13

@liustanley liustanley Awaiting requested review from liustanley

@mackjmr mackjmr Awaiting requested review from mackjmr

Assignees

@evan-bradley evan-bradley

Labels
connector/datadog exporter/datadog Datadog components processor/k8sattributes k8s Attributes processor
Projects
None yet
Milestone
v0.114.0
Development

Successfully merging this pull request may close these issues.

[processor/k8sattributes] Add support for profiles signal
7 participants
@haoqixu @dmitryax @dmathieu @mx-psi @ChrsMark @TylerHelmuth @evan-bradley