* Remove NewPossibleNonCounterInfo until it can be made more efficient, and avoid creating empty annotations as much as possible
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Return annotations (warnings and infos) from PromQL queries
This generalizes the warnings we have already used before (but only for problems with remote read) as "annotations".
Annotations can be warnings or infos (the latter could be false positives). We do not treat them different in the API for now and return them all as "warnings". It would be easy to distinguish them and return infos separately, should that appear useful in the future.
The new annotations are then used to create a lot of warnings or infos during PromQL evaluations. Partially these are things we have wanted for a long time (e.g. inform the user that they have applied `rate` to a metric that doesn't look like a counter), but the new native histograms have created even more needs for those annotations (e.g. if a query tries to aggregate float numbers with histograms).
The annotations added here are not yet complete. A prominent example would be a warning about a range too short for a rate calculation. But such a warnings is more tricky to create with good fidelity and we will tackle it later.
Another TODO is to take annotations into account when evaluating recording rules.
---------
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Using github.com/klauspost/compress package to replace the current Gzip Handler on the API.
We see significant improvements using this handler over the current one as shown in the benchmark added.
Also:
* move selection of compression from `newCompressedResponseWriter` to `*CompressionHandler.ServeHTTP`.
* renaming `compressedResponseWriter` since it now only does one kind of compression.
Signed-off-by: Alan Protasio <alanprot@gmail.com>
Using github.com/klauspost/compress package to replace the current Gzip Handler on the API.
We see significant improvements using this handler over the current one as shown in the benchmark added.
Also:
* move selection of compression from `newCompressedResponseWriter` to `*CompressionHandler.ServeHTTP`.
* renaming `compressedResponseWriter` since it now only does one kind of compression.
Signed-off-by: Alan Protasio <alanprot@gmail.com>
This reverts the removal of type casting due to an error in the
dragonfly integration. The change in the type casting introduced by the
commit causes a type mismatch, resulting in the following errors:
util/runtime/limits_default.go:42:57: cannot use rlimit.Cur (variable of type int64) as type uint64 in argument to limitToString
util/runtime/limits_default.go:42:90: cannot use rlimit.Max (variable of type int64) as type uint64 in argument to limitToString
Reverting this commit to resolve the type mismatch error and maintain compatibility with the dragonfly integration.
Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
Wiser coders than myself have come to the conclusion that a `switch`
statement is almost always superior to a statement that includes any
`else if`.
The exceptions that I have found in our codebase are just these two:
* The `if else` is followed by an additional statement before the next
condition (separated by a `;`).
* The whole thing is within a `for` loop and `break` statements are
used. In this case, using `switch` would require tagging the `for`
loop, which probably tips the balance.
Why are `switch` statements more readable?
For one, fewer curly braces. But more importantly, the conditions all
have the same alignment, so the whole thing follows the natural flow
of going down a list of conditions. With `else if`, in contrast, all
conditions but the first are "hidden" behind `} else if `, harder to
spot and (for no good reason) presented differently from the first
condition.
I'm sure the aforemention wise coders can list even more reasons.
In any case, I like it so much that I have found myself recommending
it in code reviews. I would like to make it a habit in our code base,
without making it a hard requirement that we would test on the CI. But
for that, there has to be a role model, so this commit eliminates all
`if else` occurrences, unless it is autogenerated code or fits one of
the exceptions above.
Signed-off-by: beorn7 <beorn@grafana.com>
We haven't updated golint-ci in our CI yet, but this commit prepares
for that.
There are a lot of new warnings, and it is mostly because the "revive"
linter got updated. I agree with most of the new warnings, mostly
around not naming unused function parameters (although it is justified
in some cases for documentation purposes – while things like mocks are
a good example where not naming the parameter is clearer).
I'm pretty upset about the "empty block" warning to include `for`
loops. It's such a common pattern to do something in the head of the
`for` loop and then have an empty block. There is still an open issue
about this: https://github.com/mgechev/revive/issues/810 I have
disabled "revive" altogether in files where empty blocks are used
excessively, and I have made the effort to add individual
`// nolint:revive` where empty blocks are used just once or twice.
It's borderline noisy, though, but let's go with it for now.
I should mention that none of the "empty block" warnings for `for`
loop bodies were legitimate.
Signed-off-by: beorn7 <beorn@grafana.com>
* Limit FastRegexMatcher by size (bytes) and add a TTL to each entry
Signed-off-by: Marco Pracucci <marco@pracucci.com>
* Add TestNewFastRegexMatcher_CacheSizeLimit
Signed-off-by: Marco Pracucci <marco@pracucci.com>
* Tolerate ristretto goroutines when checking goroutine leaks
Signed-off-by: Marco Pracucci <marco@pracucci.com>
* Tolerate ristretto goroutines when checking goroutine leaks
Signed-off-by: Marco Pracucci <marco@pracucci.com>
---------
Signed-off-by: Marco Pracucci <marco@pracucci.com>
