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

[StatsD receiver]Add timing/histogram for statsD receiver as OTLP gauge #2973

Merged
merged 1 commit into from
Apr 12, 2021

Conversation

gavindoudou
Copy link
Contributor

Description:
This PR is for timing/histogram support using OTLP gauge without aggregation. StatsD receiver receives ms or h StatsD type and transfers to OTLP double gauge. We discussed this approach with community last month. Timing/histogram use OTLP gauge instead of OTLP histogram, because:

  • Other preferred downstream AWS EMF exporter requires raw data.
  • Histogram OTLP type is changing.

I will implement timing/histogram using OTLP summary data type in the next PR. The user could choose using gauge or summary in config file after the next PR is ready.

Link to tracking Issue:
#2566

Testing:
Added unit tests.

Documentation:

@gavindoudou gavindoudou requested a review from jmacd as a code owner April 1, 2021 16:59
@gavindoudou gavindoudou requested a review from a team April 1, 2021 16:59
@gavindoudou gavindoudou force-pushed the statsDDoubleGaugeTimer branch 2 times, most recently from e6de5aa to e29c81f Compare April 1, 2021 18:51
@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #2973 (d378f42) into main (736647a) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2973      +/-   ##
==========================================
+ Coverage   91.55%   91.57%   +0.02%     
==========================================
  Files         465      467       +2     
  Lines       22848    22904      +56     
==========================================
+ Hits        20918    20974      +56     
  Misses       1437     1437              
  Partials      493      493              
Flag Coverage Δ
integration 68.96% <ø> (ø)
unit 90.54% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
receiver/statsdreceiver/config.go 100.00% <100.00%> (ø)
receiver/statsdreceiver/factory.go 100.00% <100.00%> (ø)
receiver/statsdreceiver/protocol/statsd_parser.go 98.48% <100.00%> (+0.33%) ⬆️
receiver/statsdreceiver/protocol/utils.go 100.00% <100.00%> (ø)
receiver/statsdreceiver/receiver.go 92.98% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 736647a...d378f42. Read the comment docs.

@gavindoudou gavindoudou force-pushed the statsDDoubleGaugeTimer branch 2 times, most recently from 925be24 to e2108fd Compare April 1, 2021 19:24
receiver/statsdreceiver/README.md Outdated Show resolved Hide resolved
receiver/statsdreceiver/protocol/statsd_parser.go Outdated Show resolved Hide resolved
@jmacd
Copy link
Contributor

jmacd commented Apr 5, 2021

Let me know if this forward-looking configuration support plan seems to much of a burden. I hope it's not! The functionality looks 💯.

@gavindoudou gavindoudou force-pushed the statsDDoubleGaugeTimer branch 6 times, most recently from 9c4291a to 44b91fd Compare April 7, 2021 18:39
@gavindoudou gavindoudou requested a review from jmacd April 7, 2021 19:08
Copy link
Contributor

@JohnWu20 JohnWu20 left a comment

Choose a reason for hiding this comment

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

LGTM

receiver/statsdreceiver/README.md Outdated Show resolved Hide resolved
receiver/statsdreceiver/README.md Outdated Show resolved Hide resolved
receiver/statsdreceiver/README.md Outdated Show resolved Hide resolved
receiver/statsdreceiver/README.md Outdated Show resolved Hide resolved
receiver/statsdreceiver/README.md Outdated Show resolved Hide resolved
receiver/statsdreceiver/protocol/statsd_parser.go Outdated Show resolved Hide resolved
@gavindoudou gavindoudou force-pushed the statsDDoubleGaugeTimer branch 2 times, most recently from 69883df to 69feb73 Compare April 8, 2021 22:42
@gavindoudou gavindoudou force-pushed the statsDDoubleGaugeTimer branch from 69feb73 to d378f42 Compare April 8, 2021 22:54
@bogdandrutu bogdandrutu merged commit a63832b into open-telemetry:main Apr 12, 2021
@gavindoudou gavindoudou deleted the statsDDoubleGaugeTimer branch April 27, 2021 19:17
pmatyjasek-sumo pushed a commit to pmatyjasek-sumo/opentelemetry-collector-contrib that referenced this pull request Apr 28, 2021
punya pushed a commit to punya/opentelemetry-collector-contrib that referenced this pull request Jul 21, 2021
* Change With*Unmarshallers signatures; Rename variables in kafkaReceiverFactory

* fix tests

* rollback change to variables in kafkaReceiverFactory
alexperez52 referenced this pull request in open-o11y/opentelemetry-collector-contrib Aug 18, 2021
* Change With*Unmarshallers signatures; Rename variables in kafkaReceiverFactory

* fix tests

* rollback change to variables in kafkaReceiverFactory
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.

5 participants