pkg/line: support DogStatsD v1.3 metric timestamp field (|T<unix_ts>)#684
Open
alliasgher wants to merge 1 commit into
Open
pkg/line: support DogStatsD v1.3 metric timestamp field (|T<unix_ts>)#684alliasgher wants to merge 1 commit into
alliasgher wants to merge 1 commit into
Conversation
DogStatsD protocol v1.3 adds an optional metric timestamp suffix: <name>:<value>|<type>|#<tags>|T<unix_timestamp> The timestamp is a UTC Unix epoch integer, e.g. T1656581400. statsd_exporter does not propagate source timestamps to Prometheus, so the field is parsed for validity and then discarded — this matches the behaviour of the container-ID (|c:) field introduced in v1.2. Changes: - Add a 'T' case in the component switch to accept and validate the timestamp field; invalid or missing values increment the malformed_timestamp error counter. - Raise the max-components guard from 5 to 8 to accommodate all v1.3 fields (value, type, @sample, #tags, |c:container, |T<ts>) without triggering a false malformed_component error. - Add five test cases: counter with timestamp, gauge with timestamp, combined sampling+container+timestamp, and two malformed cases. Fixes prometheus#680 Signed-off-by: Ali <alliasgher123@gmail.com>
Contributor
pedro-stanaka
left a comment
There was a problem hiding this comment.
Been busy times for me, finally had the chance to review this. Need some clarification and I think we can drop the expensive parse of the timestamp.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DogStatsD protocol v1.3 adds an optional timestamp suffix to the metric datagram:
Previously any datagram containing this field was silently dropped because
Twas not a recognised component prefix (hit thedefaultbranch →continue samples) and the 5-component guard rejected fully-formed v1.3 datagrams.Changes
pkg/line/line.go: add a'T'case in the component switch that validates the Unix timestamp integer and ignores it (statsd_exporter does not propagate source timestamps to Prometheus — same approach as the v1.2|c:<container_id>field)5→8to accommodate all six possible v1.3 fields (value,type,@sample,#tags,|c:,|T:) without hitting a falsemalformed_componenterrorpkg/line/line_test.go: five new test cases — counter, gauge, combined sampling+container+timestamp, and two malformed-timestamp casesTests
Fixes #680