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

feat(processors/k8sattributesprocessor): support metadata enrichment based on multiple attributes #8465

Merged
merged 38 commits into from
Jul 5, 2022
Merged

feat(processors/k8sattributesprocessor): support metadata enrichment based on multiple attributes #8465

merged 38 commits into from
Jul 5, 2022

Conversation

sumo-drosiek
Copy link
Member

Description:

This PR is continuation of #4310 as I cannot reopen it.

It deprecates current pod association structure and instead it proposes another one, which is able to handle multiple source attributes. The direct justification is to support metadata enrichment for pair (k8s.pod.name, k8s.namespace.name).

Link to tracking Issue: #4309

Testing: unit tests

Documentation: doc.go

@sumo-drosiek sumo-drosiek requested a review from a team March 15, 2022 07:37
@jpkrohling jpkrohling changed the title feat(processors/k8sattribtuesprocessor): support metadata enrichment based on multiple attributes feat(processors/k8sattributesprocessor): support metadata enrichment based on multiple attributes Mar 15, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2022

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Apr 6, 2022
@sumo-drosiek sumo-drosiek force-pushed the drosiek-k8sprocessor branch from 4e0efdd to bd0a3f0 Compare April 11, 2022 14:00
@pmm-sumo
Copy link
Contributor

I believe the items were addressed and this is ready for final review. @sumo-drosiek could you resolve the conflicts (there were some additional changes in the meantime)

@dmitryax can you have another look?

@sumo-drosiek sumo-drosiek force-pushed the drosiek-k8sprocessor branch from bd0a3f0 to 66afd97 Compare April 13, 2022 10:49
@github-actions github-actions bot removed the Stale label Apr 14, 2022
@newly12
Copy link
Contributor

newly12 commented Apr 19, 2022

Hi all, how can we move forward on this PR?

dmitryax
dmitryax reviewed Apr 20, 2022
View reviewed changes
Copy link
Member

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

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

Sorry for the delayed review

processor/k8sattributesprocessor/internal/kube/client.go Outdated Show resolved Hide resolved
processor/k8sattributesprocessor/pod_association.go Outdated Show resolved Hide resolved
@sumo-drosiek sumo-drosiek force-pushed the drosiek-k8sprocessor branch 3 times, most recently from d66325c to 17c410d Compare April 22, 2022 12:13
@sumo-drosiek
Copy link
Member Author

sumo-drosiek commented Apr 25, 2022 •
edited
Loading

@dmitryax Sorry for number of iterations. I cleaned up the code according to the most recent comments

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 13, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: sumo-drosiek / name: Dominik Rosiek (934f2b013afce5fc612ae45b866df765a077bba5, b89a44553302285fd25f9d735aa29636c3820e9f, 31270caf343554a2da386619334cfb344ff62ea7, fd7f7e55f073b9ffa64c44db314db078c929b1bb, 671810620b55cdc3a3b406252504ecb6b863cc57, 446580389544c54297a9942bbbc22421a47ee43b, de71e23e431ef9a8aa524d98f8c86a87498c2046, a6289aa4586c251305ebeaeceefd2439471527b7, 3adede9c8a2237b865025a8b05f0778c6c7c5c12, 9af151309965b216b4ad4610afb771c1b1b16ce8, 0a21aa9740a98c03384ab53041714268b42b0a45, 3de3dd75f3eded7edc0231e1a31709153e83c258, 9752ac7ae4cc3aa94dc023c754fcc4f5053b2683, 5cd1a7ff29ca3dce4c27cb3c21aa7235cbfa1ca3, c2a2abb6a4750887fda4788988c5c7bd0ea17ca8, 28ed3007a385fd4983ce07a03669d94eb37a2dd7, ebc4e7d582e6abed4408124a32743acbd1afb6c2, 2f51fb61fcc77a241922be35a66b6f3118c71998, 36e5d7d52dfc09218185f14e487ea4557e303b2f, 7ef65c26eef0343457a1f7f4bce7ec409c811d1b, 44765144dea5a80d716271e6a60fb7bc550a4ca8, 95024f6cb0069bae2caa57a03acab868bbdd4de9, 86e7fb0b4113337c9740792113eda2a448a6b6e8, c301b829c44a6848b58605c72af958f534bb0be2, 207c940e8561938fcd6682ef1877cf08f5cd0ff8, 70c0ac9b14088632fbffeef9672db5c87fd01ba5, df85eebf7ad525b5c0cb9f6554a214630610a2c0, 414b3e64ff4144c887418afe488da91f350680a6, daae3bf6d048b7b5a8548d8c47779b72afd52244, 17c410d687e54540247833e606c027b280add309, e30f69d5596e8dc3e719367fdbf83e555272fdb5, 92160f552a0bab33ced4547681392b0ca818dd4d, 7cc604003f40b5d637caad4035ca770634a52e65, c3f0147ca5f902ec612b091063a8c06fde0880cb)
  • ✅ login: Wolodija / name: Dominik (1bb8250cf4479b89c2be872230fdeb8f7351c471)

Dominik Rosiek and others added 18 commits July 4, 2022 07:53
refactor(processors/k8sattributes): change PodIdentifier type
daafea1 
Signed-off-by: Dominik Rosiek <[email protected]>
refactor(k8sattributesprocessor): change getIdentifiersFromAssoc's ne…
3c3f4a9 
…wPod to pod

Signed-off-by: Dominik Rosiek <[email protected]>
fix(k8sattributesprocessor): do not use source.Name for connection so…
ee507b9 
…urce.From

Signed-off-by: Dominik Rosiek <[email protected]>
refactor(k8sattributesprocessor): update deprecation log
c5e30fc 
Signed-off-by: Dominik Rosiek <[email protected]>
feat(k8sattributesprocessor): reduce PodIdentifierMaxLength to 4
3e35a2f 
Signed-off-by: Dominik Rosiek <[email protected]>
refactor(k8sattributesprocessor): rename GetPodIdentifierAttribute to…
33f60a4 
… PodIdentifierAttributeFromSource

Signed-off-by: Dominik Rosiek <[email protected]>
refactor(k8sattributesprocessor): extract resource_attribute and conn…
ea77bf8 
…ection as consts

Signed-off-by: Dominik Rosiek <[email protected]>
refactor(k8sattributesprocessor): do not use name for connection at all
6321755 
Signed-off-by: Dominik Rosiek <[email protected]>
docs(processor/k8sattributeprocessor): udpate
875c218 
Signed-off-by: Dominik Rosiek <[email protected]>
feat(processors/k8sattributes): fix after rebase
d8125a3 
Signed-off-by: Dominik Rosiek <[email protected]>
refactor(k8sattributesprocessor): clean up asso.Name related code
e44bf48 
Signed-off-by: Dominik Rosiek <[email protected]>
refactor: refactor extractPodIDNoAssociations
6ab9edc 
Signed-off-by: Dominik Rosiek <[email protected]>
refactor: return PodIdentifierAttribute for getConnectionIP
f8eb928 
Signed-off-by: Dominik Rosiek <[email protected]>
@macdewee
refactor: review
bbef85e 
Signed-off-by: Dominik Rosiek <[email protected]>
@macdewee
fix: validation of empty podIdentifier
bdfc814 
Signed-off-by: Dominik Rosiek <[email protected]>
@sumo-drosiek @dmitryax
Update processor/k8sattributesprocessor/factory.go
0efc3a7 
Co-authored-by: Dmitrii Anoshin <[email protected]>
@sumo-drosiek @dmitryax
Update processor/k8sattributesprocessor/factory.go
fac8f7e 
Co-authored-by: Dmitrii Anoshin <[email protected]>
refactor: clean up
54d1ccb 
Signed-off-by: Dominik Rosiek <[email protected]>
@sumo-drosiek sumo-drosiek force-pushed the drosiek-k8sprocessor branch from 258829c to 54d1ccb Compare July 4, 2022 06:01
Dominik Rosiek added 2 commits July 4, 2022 08:03
chore(changelog): update
b80ffbe 
Signed-off-by: Dominik Rosiek <[email protected]>
refactor: clean up
433caca 
Signed-off-by: Dominik Rosiek <[email protected]>
@dmitryax dmitryax merged commit 4cf86de into open-telemetry:main Jul 5, 2022
@dmitryax dmitryax mentioned this pull request Jul 19, 2022
Merged
@rajatvig rajatvig mentioned this pull request Aug 9, 2022
Closed
@evantorrie evantorrie mentioned this pull request Aug 31, 2022
Closed
@dmitryax dmitryax mentioned this pull request Mar 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@newly12 newly12 newly12 left review comments

@dmitryax dmitryax dmitryax approved these changes

@pmm-sumo pmm-sumo pmm-sumo approved these changes

Assignees

@Aneurysm9 Aneurysm9

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sumo-drosiek @pmm-sumo @newly12 @macdewee @dmitryax @Aneurysm9