Break out the telemetry during sample processing #106
Merged
Conversation
This file contains 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
The "packets" metric had heavily overloaded meaning for different "outcomes", and would often be incremented multiple times, sometimes even with a single (per-line) increment in one outcome corresponding to multiple increments in another. This removes the broken metric, and replaces it with separate total and error counters for each level of processing. This allows monitoring the network traffic handled separately from the samples incurred by it.
Closed
|
@bakins WDYT about getting this into the 0.5.0 release? I'd either merge it now & then you can rebase the release branch, or I hold it back for the next release. it's technically another breaking chagne, and I kinda prefer bunching them up, OTOH the old metric was so bad I'm not too worried about breaking it. |
|
@matthiasr is fine with me to include in 0.5.0 |
grobie
approved these changes
Nov 14, 2017
| log.Debugf("Read %s failed: %v", c.RemoteAddr(), err) | ||
| } | ||
| break | ||
| } | ||
| if isPrefix { | ||
| networkStats.WithLabelValues("tcp_line_too_long").Inc() | ||
| tcpLineTooLong.Inc() |
There was a problem hiding this comment.
Uhm, we should just fix the reader though, no need to abort on a long line. https://golang.org/pkg/bufio/#Reader.ReadLine
|
Yes, this should be really easy to fix, but this PR is about the metrics. |
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.
The "packets" metric had heavily overloaded meaning for different
"outcomes", and would often be incremented multiple times, sometimes
even with a single (per-line) increment in one outcome corresponding to
multiple increments in another.
This removes the broken metric, and replaces it with separate total and
error counters for each level of processing. This allows monitoring the
network traffic handled separately from the samples incurred by it.
Supersedes #54 based on the discussion there.