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/attributesprocessor] Add metric support #8111

Merged
merged 10 commits into from
Mar 10, 2022

Conversation

crobert-1
Copy link
Member

Description:

This change adds metric support to the attributes processor. Metric data point attributes can now be modified in all of the same ways as spans and logs.

Link to tracking Issue:

Testing:

Included unit tests, also did manual testing. Manual tests show that insert, delete and extract were all working as expected.
Documentation:

README was updated, as well as extra comments in shared attributes processor code.

Copy link
Member

@pmcollins pmcollins left a comment

Choose a reason for hiding this comment

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

Fine work -- just a few questions.

processor/attributesprocessor/README.md Show resolved Hide resolved
processor/attributesprocessor/README.md Outdated Show resolved Hide resolved
This change adds metric support to the attributes processor. Metric
data point attributes can now be modified in all of the same ways
as spans and logs. This change also adds testing that is all passing.
@crobert-1 crobert-1 requested a review from punya as a code owner March 2, 2022 23:45
@crobert-1 crobert-1 force-pushed the metric_attribute_processor branch 4 times, most recently from c64a217 to 230455f Compare March 3, 2022 16:48
- Remove out-dated comments
- Change all span/log/metric references to "input data"
- Add comments about usage of attributes processor vs.
metric transform processor.
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.

LGTM

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

@pmm-sumo @pmcollins @tigrannajaryan please take a look and approve if your comments have been addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants