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

Add parsing for labels and guages in statsdreceiver #903

Merged
merged 7 commits into from
Sep 9, 2020
Merged

Add parsing for labels and guages in statsdreceiver #903

merged 7 commits into from
Sep 9, 2020

Conversation

sonofachamp
Copy link
Contributor

@sonofachamp sonofachamp commented Sep 2, 2020

Description:

Add parsing for gauges and labels.

Link to tracking Issue:
#290

Testing:
Expanded unit test coverage.

Documentation:

@sonofachamp sonofachamp requested a review from a team September 2, 2020 21:50
@tigrannajaryan
Copy link
Member

@sonofachamp is this fixing an open issue?

@sonofachamp
Copy link
Contributor Author

@tigrannajaryan Not fixing an open issue, but expanding on the StatsD receiver support #290

Copy link
Member

@asuresh4 asuresh4 left a comment

Choose a reason for hiding this comment

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

Mostly looks good, left a couple of comments.

Also, please rebase to fix lint.

receiver/statsdreceiver/protocol/statsd_parser_test.go Outdated Show resolved Hide resolved
receiver/statsdreceiver/protocol/statsd_parser.go Outdated Show resolved Hide resolved
receiver/statsdreceiver/protocol/statsd_parser_test.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 4, 2020

Codecov Report

Merging #903 into master will increase coverage by 0.04%.
The diff coverage is 96.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #903      +/-   ##
==========================================
+ Coverage   88.47%   88.52%   +0.04%     
==========================================
  Files         245      245              
  Lines       13085    13149      +64     
==========================================
+ Hits        11577    11640      +63     
- Misses       1148     1149       +1     
  Partials      360      360              
Flag Coverage Δ
#integration 74.88% <ø> (ø)
#unit 87.73% <96.73%> (+0.05%) ⬆️

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

Impacted Files Coverage Δ
receiver/statsdreceiver/protocol/statsd_parser.go 97.32% <96.73%> (+1.48%) ⬆️

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 faaf8d3...0392261. Read the comment docs.

@bogdandrutu bogdandrutu merged commit 491ecb6 into open-telemetry:master Sep 9, 2020
@sonofachamp sonofachamp deleted the labels-and-sample-rate branch September 9, 2020 16:04
@lubingfeng
Copy link

Good to see this is merged. Thanks.
@bogdandrutu @jmacd Just want to check what we need to do and how to test to ensure this statsD receiver is using OTLP compliant protocol.

@lubingfeng
Copy link

lubingfeng commented Sep 10, 2020

@jmacd thanks for the input over today's discussion. As suggested, we will consider writing one histogram per point for now.

Questions for aggregation:

  1. Every metrics, no matter it's an counter or an timing, to the histogram.
  2. What should be collection interval - not customizable, user defined, or receiver specific, given 1 second aggregation interval?

I am more than happy to have a separate call this week to get this sorted out before our implementation.

dyladan referenced this pull request in dynatrace-oss-contrib/opentelemetry-collector-contrib Jan 29, 2021
I chatted with sjkaris and he is currently not going to actively
contribute to the Collector so he will not need the "approver" role.
Removing for now and will add back when he decides to contribute
to the Collector again.

Thanks, sjkaris!
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* Avoid applying stale udpates; add a test

* Add Memory option to basic processor

* Always use memory in the pull controller

* Test the memory option

* Precommit

* Add a Prometheus-specific test

* More comment on Memory option

* Link to 862

* Remove sleep

* Update changelog

* Comment on stale and stateless aggregators

* Update sdk/metric/processor/basic/config.go

Co-authored-by: Tyler Yahn <[email protected]>

Co-authored-by: Liz Fong-Jones <[email protected]>
Co-authored-by: Tyler Yahn <[email protected]>
codeboten pushed a commit that referenced this pull request Nov 23, 2022
… adding 'attributes' parameter to util function (#903)

* code changes to resolve conditional server span creation for WSGI (open-telemetry/opentelemetry-python-contrib#454)

* Adding entry to changelog.md

* modifying _start_internal_or_server_span() to add attributes as a parameter. Also calling _start_internal_or_server_span() in WSGI instrumentation

* resolving flake8 and typo issues
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