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

[sumconnector] implement summing logic #34797

Conversation

greatestusername
Copy link
Contributor

@greatestusername greatestusername commented Aug 21, 2024

Description:

  • Adds connector and summing logic
  • Adds testing of traces and spans summing

Note: testing and test data makes up the bulk of the lines in this PR

Link to tracking Issue: 32669

Testing:

  • condition, attribute, default attribute, multiple condition, multiple attributes, multiple metrics for traces / spans telemetry types

Documentation: No new docs

Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

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

If it's not too much trouble, could we split this PR into three: one for each of metrics, traces, and logs?

This was a pretty minimal review, if we're able to break it up it would be really helpful in being able to give a more detailed review.

connector/sumconnector/go.mod Outdated Show resolved Hide resolved
connector/sumconnector/go.mod Outdated Show resolved Hide resolved
connector/sumconnector/metadata.yaml Outdated Show resolved Hide resolved
connector/sumconnector/sum.go Outdated Show resolved Hide resolved
.chloggen/32669-sumconnector-sum-logic.yaml Outdated Show resolved Hide resolved
connector/sumconnector/connector.go Outdated Show resolved Hide resolved
connector/sumconnector/sum.go Show resolved Hide resolved
@greatestusername greatestusername marked this pull request as ready for review September 4, 2024 16:22
@github-actions github-actions bot added the cmd/otelcontribcol otelcontribcol command label Sep 4, 2024
cmd/otelcontribcol/go.mod Outdated Show resolved Hide resolved
connector/sumconnector/README.md Outdated Show resolved Hide resolved
.chloggen/32669-sumconnector-sum-logic.yaml Outdated Show resolved Hide resolved
connector/sumconnector/connector.go Show resolved Hide resolved
connector/sumconnector/sum.go Outdated Show resolved Hide resolved
Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

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

Just some whitespace nits, looks good though. Thanks for adding the pmetric ignore test condition too!

connector/sumconnector/README.md Outdated Show resolved Hide resolved
connector/sumconnector/README.md Outdated Show resolved Hide resolved
connector/sumconnector/README.md Outdated Show resolved Hide resolved
@greatestusername
Copy link
Contributor Author

Thanks for the approval! README whitespace nits addressed.

@greatestusername
Copy link
Contributor Author

@crobert-1 Is there anyone else I need approvals from or just waiting for someone to merge in the next round?
Thanks!

@crobert-1 crobert-1 added the ready to merge Code review completed; ready to merge by maintainers label Sep 17, 2024
@crobert-1
Copy link
Member

Is there anyone else I need approvals from or just waiting for someone to merge in the next round?

We should be good, I've added ready to merge. 👍

@greatestusername
Copy link
Contributor Author

Thanks!!!

@greatestusername greatestusername requested a review from a team as a code owner September 20, 2024 15:51
@andrzej-stencel andrzej-stencel changed the title 32669 sumconnector sum logic [sumconnector] implement summing logic Sep 24, 2024
@andrzej-stencel andrzej-stencel merged commit 68f3d7d into open-telemetry:main Sep 26, 2024
156 checks passed
@github-actions github-actions bot added this to the next release milestone Sep 26, 2024
jriguera pushed a commit to springernature/opentelemetry-collector-contrib that referenced this pull request Oct 4, 2024
**Description:** 
- Adds connector and summing logic
- Adds testing of traces and spans summing

Note: testing and test data makes up the bulk of the lines in this PR

**Link to tracking Issue:** 32669

**Testing:** 
- condition, attribute, default attribute, multiple condition, multiple
attributes, multiple metrics for traces / spans telemetry types

**Documentation:** No new docs

---------

Co-authored-by: Antoine Toulme <antoine@toulme.name>
mx-psi pushed a commit that referenced this pull request Oct 7, 2024
…ata) (#35434)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Adds tests for metrics to metrics and logs to metrics
follows this pr:
#34797

This is a lot of lines but it is mostly repetition and uses the same
standard structure as the previously merged trace to metric tests.

I dont think this needs any changelog but let me know if it does for
some reason! 😸

**Link to tracking Issue:** #32669 

**Testing:** <Describe what testing was performed and which tests were
added.>
Adds testing for metrics to metrics and logs to metrics using same
methodology as previous PR

**Documentation:** No changes to docs. This is just tests.

---------

Co-authored-by: Curtis Robert <crobert@splunk.com>
Eromosele-SM pushed a commit to sematext/opentelemetry-collector-contrib that referenced this pull request Oct 9, 2024
…ata) (open-telemetry#35434)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Adds tests for metrics to metrics and logs to metrics
follows this pr:
open-telemetry#34797

This is a lot of lines but it is mostly repetition and uses the same
standard structure as the previously merged trace to metric tests.

I dont think this needs any changelog but let me know if it does for
some reason! 😸

**Link to tracking Issue:** open-telemetry#32669 

**Testing:** <Describe what testing was performed and which tests were
added.>
Adds testing for metrics to metrics and logs to metrics using same
methodology as previous PR

**Documentation:** No changes to docs. This is just tests.

---------

Co-authored-by: Curtis Robert <crobert@splunk.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd/otelcontribcol otelcontribcol command connector/sum ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants