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

Remove the MinMaxSumCount from cortex and datadog exporter #1554

Merged

Conversation

hanyuancheung
Copy link
Member

@hanyuancheung hanyuancheung commented Jan 6, 2022

This is part of the #1478 todo list:

  • Remove the MinMaxSumCount from cortex.
  • Remove Point aggregations from datadog exporter.
  • Handle Histograms in the datadog exporter
  • Remove Point aggregations from dogstatsd exporter.
  • Handle Histograms in the dogstatsd exporter

@codecov
Copy link

codecov bot commented Jan 6, 2022

Codecov Report

Merging #1554 (e0238ca) into main (576a237) will decrease coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #1554     +/-   ##
=======================================
- Coverage   69.3%   69.3%   -0.1%     
=======================================
  Files        127     127             
  Lines       5486    5424     -62     
=======================================
- Hits        3806    3760     -46     
+ Misses      1533    1521     -12     
+ Partials     147     143      -4     
Impacted Files Coverage Δ
exporters/metric/cortex/cortex.go 73.9% <ø> (+0.5%) ⬆️
exporters/metric/datadog/datadog.go 47.3% <ø> (-8.2%) ⬇️

Copy link
Contributor

@dashpole dashpole left a comment

Choose a reason for hiding this comment

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

lgtm after nit is addressed

Copy link
Member

@Aneurysm9 Aneurysm9 left a comment

Choose a reason for hiding this comment

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

This also appears to remove MMSC from the datadog exporter. Please update the PR title and CHANGELOG.

@hanyuancheung hanyuancheung changed the title Remove the MinMaxSumCount from cortex Remove the MinMaxSumCount from cortex and datadog exporter Jan 7, 2022
@hanyuancheung
Copy link
Member Author

This also appears to remove MMSC from the datadog exporter. Please update the PR title and CHANGELOG.

All done. PTAL @Aneurysm9 @dashpole

exporters/metric/datadog/example_test.go Outdated Show resolved Hide resolved
@hanyuancheung hanyuancheung force-pushed the remove-MinMaxSumCount-from-cortex branch from 360222a to 7c80254 Compare January 8, 2022 12:39
@hanyuancheung hanyuancheung force-pushed the remove-MinMaxSumCount-from-cortex branch from 7c80254 to ba223ad Compare January 8, 2022 12:58
@hanyuancheung hanyuancheung force-pushed the remove-MinMaxSumCount-from-cortex branch from 3b4d329 to e0238ca Compare January 8, 2022 13:34
@hanyuancheung
Copy link
Member Author

Thanks for the suggestions, PTAL.

@Aneurysm9 Aneurysm9 merged commit 390c3c6 into open-telemetry:main Jan 10, 2022
@MrAlias MrAlias mentioned this pull request Feb 11, 2022
plantfansam referenced this pull request in plantfansam/opentelemetry-go-contrib Mar 18, 2022
Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
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.

None yet

4 participants