-
Notifications
You must be signed in to change notification settings - Fork 244
DogStatsD v1.2 Container ID support with sampling support #649
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
base: master
Are you sure you want to change the base?
DogStatsD v1.2 Container ID support with sampling support #649
Conversation
Signed-off-by: Marc Harrison <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for DogStatsD v1.2 container ID extension, allowing metrics to include container ID information via a |c:<container_id> suffix. The implementation extends the existing parser to handle up to 5 components (increased from 4) and adds parsing logic for the container ID section.
- Increased component limit from 4 to 5 to accommodate the new container ID field
- Added parsing logic for the
|c:<container_id>format with proper validation - Enhanced test coverage for various container ID scenarios including sampling, different metric types, and malformed inputs
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/line/line.go | Implements container ID parsing logic and increases component limit |
| pkg/line/line_test.go | Adds comprehensive test cases for container ID functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Signed-off-by: Marc Harrison <[email protected]>
b1f9092 to
f68b77b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks mostly good, the log line will show the wrong message, if I am not mistaken, no?
Signed-off-by: Marc Harrison <[email protected]>
c8bee20 to
3493551
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for the contribution.
Fixes #640.