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>
This makes it easier to connect a log message with the config it relates
to.
Each SD config has a name, either the scrape job name or something like
"config-0" for Alertmanager config.
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
While originally the resync period also forced refreshing from Kubernetes API server, this has been removed for some years now because watching the API server got more stable [1]. Today, this just results in all entities being sent to the service discovery again, which is valid from a general Prometheus perspective, but results in unnecessary CPU load and also breaks service discovery metrics. In especially, this makes monitoring "do we actually observe changes from Kubernetes API server" impossible (receiving constant updates from Kubernetes service discovery is a pretty valid assumption, for example nodes get frequent status updates, ...).
Signed-off-by: Jens Erat <jens.erat@mercedes-benz.com>
* Add VM size label to azure service discovery (#11575)
Signed-off-by: davidifr <davidfr.mail@gmail.com>
* Add VM size label to azure service discovery (#11575)
Signed-off-by: davidifr <davidfr.mail@gmail.com>
* Add VM size label to azure service discovery (#11575)
Signed-off-by: davidifr <davidfr.mail@gmail.com>
Signed-off-by: davidifr <davidfr.mail@gmail.com>
A new API is available for AddEventHandlers, to get errors but also be
able to cancel handlers.
Doing the easy thing for the release, which is just to log errors.
We could see how to improve this in the future to handle the errors
properly and cancel the handlers.
Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
* Common client in EC2 and Lightsail
Signed-off-by: Levi Harrison <git@leviharrison.dev>
* Azure -> AWS
Signed-off-by: Levi Harrison <git@leviharrison.dev>
Signed-off-by: Levi Harrison <git@leviharrison.dev>
* file sd: create and increment an inotify error counter when file-SD is unable to watch files. Additionally, order metrics alphabetically.
Signed-off-by: Michael Fuller <mfuller@digitalocean.com>
* file.go: consistent naming and help for prometheus_sd_file_watcher_errors_total
Signed-off-by: Michael Fuller <mfuller@digitalocean.com>
Signed-off-by: Michael Fuller <mfuller@digitalocean.com>
Co-authored-by: Michael Fuller <mfuller@digitalocean.com>
Function arguments in defer evaluated during definition of defer, not
during execution
Signed-off-by: Slavik Panasovets <slavik@google.com>
Signed-off-by: Slavik Panasovets <slavik@google.com>
* Update go to 1.19, set min version to 1.18
Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
* Update golangci-lint
Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
This fixes an occurrence of a loop variable being captured in a
parallel test (`TestInitialUpdate`). Prior to this commit, only the
last test case declared in that test would actually execute. To work
around this problem, we create a copy of the range variable before the
paralllel test, as suggested in the documentation for the `testing`
package:
https://pkg.go.dev/testing#hdr-Subtests_and_Sub_benchmarks
The test immediately after the one fixed here (`TestInvalidFile`)
followed the same pattern but correctly created a copy of the loop
variable, illustrating how easy it is to forget to do this in
practice.
Issue was automatically found using the `loopvarcapture` linter.
Signed-off-by: Renato Costa <renato@cockroachlabs.com>
Signed-off-by: Renato Costa <renato@cockroachlabs.com>
It's currently possible to use blackbox_exporter to probe MX records
themselves. However it's not possible to do an end-to-end test, like is
possible with SRV records. This makes it possible to use MX records as a
source of hostnames in the same way as SRV records.
Signed-off-by: David Leadbeater <dgl@dgl.cx>
This commits adds a __meta_kubernetes_pod_container_image as a new
metadata label. This can be used to alert on mismatched versions of
targets who don't have a build_info metric, as well as injecting it into
log lines for other consumers of discovery/kubernetes (e.g., Promtail).
Signed-off-by: Robert Fratto <robertfratto@gmail.com>
The Kubernetes service discovery can only add node labels to
targets from the pod role.
This commit extends this functionality to the endpoints and
endpointslices roles.
Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
The tests for Kubernetes SD rely on comparing target groups by first
serializing them to JSON. However, the target group MarshalJSON function
only serializes the __address__ label, which makes eliminates all other
labels from the comparison.
This commit implements a separate marshaling function intended for use in
Kubernetes SD tests. The function serializes all target labels, making
comparisons much more reliable. The commit also fixes all tests that
started to fail due to the newly introduced change.
Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
This commit introduces a new metric to count the number of failed
requests to Linode's API when using Linode SD. Resolves#10672, inspired
by #10476.
_Note_: this doens't count failures when polling the `/account/events`
endpoint, as a `401` there is how we determine if the supplied token has
the needed API scopes to do event polling vs full refreshes each
interval.
Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>
* refactor: move from io/ioutil to io and os packages
* use fs.DirEntry instead of os.FileInfo after os.ReadDir
Signed-off-by: MOREL Matthieu <matthieu.morel@cnp.fr>
* discovery: expose HTTP client options to discoverers
Signed-off-by: Robert Fratto <robertfratto@gmail.com>
* discovery/http: use HTTP client options for created client
Signed-off-by: Robert Fratto <robertfratto@gmail.com>
* scrape: use a list of HTTP client options instead of just dial context
Signed-off-by: Robert Fratto <robertfratto@gmail.com>
* discovery: rephrase comment
Signed-off-by: Robert Fratto <robertfratto@gmail.com>
* Run gofumpt on all files
Getting golangci-lint errors when building on my laptop, possibly because I have newer version of gofumpt then what it was formatted with.
Run gofumpt -w -extra on all files as it will be needed in the future anyway.
* Update golangci-lint to v1.44.2
v1.44.0 upgraded gofumpt so bumping version in CI will help keep formatting correct for everyone
* Address golangci-lint error
Getting 'error-strings: error strings should not be capitalized or end with punctuation or a newline' from revive here.
Drop new line.
Signed-off-by: Łukasz Mierzwa <l.mierzwa@gmail.com>
The upgraded client adds order=creation_date_desc to the query
parameters causing the tests to fail. Instead of checking the full URI,
just check that the path is correct.
Signed-off-by: Chris Marchbanks <csmarchbanks@gmail.com>
* discovery/targetgroup: support marshaling to JSON
targetgroup.Group is able to be marshaled to and from YAML, but not with
JSON. JSON is used for the http_sd API representation, so users
implementing the API were required to implement their own type which is
expected by the JSON unmarshaler.
Signed-off-by: Robert Fratto <robertfratto@gmail.com>
* fix lint error
Signed-off-by: Robert Fratto <robertfratto@gmail.com>
* Update discovery/targetgroup/targetgroup.go
Co-authored-by: Julius Volz <julius.volz@gmail.com>
Signed-off-by: Robert Fratto <robertfratto@gmail.com>
* s/Json/JSON
Signed-off-by: Robert Fratto <robertfratto@gmail.com>
Co-authored-by: Julius Volz <julius.volz@gmail.com>
Fail configuration unmarshalling if kubeconfig or api url are set with
"own namespace"
Only read namespace file if needed.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
When using Kubernetes service discovery on a Prometheus instance that's
not running inside Kubernetes, the creation of the service discovery
fails with a "no such file or directory" error as the special
`/var/run/secrets/kubernetes.io/serviceaccount/namespace` file is not
there. This commit moves the code that reads this file into the
if-branch where no `APIServer.URL` is given (that one basically makes
Prometheus assume it is running inside of a Kubernetes cluster).
Signed-off-by: Georg Gadinger <nilsding@nilsding.org>
When using Kubernetes service discovery on a Prometheus instance that's
not running inside Kubernetes, the creation of the service discovery
fails with a "no such file or directory" error as the special
`/var/run/secrets/kubernetes.io/serviceaccount/namespace` file is not
there. This commit moves the code that reads this file into the
if-branch where no `APIServer.URL` is given (that one basically makes
Prometheus assume it is running inside of a Kubernetes cluster).
Signed-off-by: Georg Gadinger <nilsding@nilsding.org>
This commit adds support for discovering targets from the same
Kubernetes namespace as the Prometheus pod itself. Own-namespace
discovery can be indicated by using "." as the namespace.
Fixes#9782
Signed-off-by: fpetkovski <filip.petkovsky@gmail.com>
When using Kubernetes on cloud providers, nodes will have the
spec.providerID field populated to contain the cloud provider specific
name of the EC2/GCE/... instance.
Let's expose this information as an additional label, so that it's
easier to annotate metrics and alerts to contain the cloud provider
specific name of the instance to which it pertains.
Signed-off-by: Ed Schouten <eschouten@apple.com>
* Add basic initial developer docs for TSDB
There's a decent amount of content already out there (blog posts,
conference talks, etc), but:
* when they get stale, they don't tend to get updated
* they still leave me with questions that I'ld like to answer
for developers (like me) who want to use, or work with, TSDB
What I propose is developer docs inside the prometheus
repository. Easy to find and harness the power of the community
to expand it and keep it up to date.
* perfect is the enemy of good. Let's have a base and incrementally improve
* Markdown docs should be broad but not too deep. Source code comments
can complement them, and are the ideal place for implementation details.
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* use example code that works out of the box
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* Apply suggestions from code review
Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com>
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* PR feedback
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* more docs
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* PR feedback
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* Apply suggestions from code review
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
* Apply suggestions from code review
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com>
* feedback
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* Update tsdb/docs/usage.md
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com>
* final tweaks
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* workaround docs versioning issue
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* Move example code to real executable, testable example.
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* cleanup example test and make sure it always reproduces
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* obtain temp dir in a way that works with older Go versions
Signed-off-by: Dieter Plaetinck <dieter@grafana.com>
* Fix Ganesh's comments
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
Co-authored-by: Ganesh Vernekar <15064823+codesome@users.noreply.github.com>
Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
Co-authored-by: Ganesh Vernekar <ganeshvern@gmail.com>
When running tests in parallel, 10 milliseconds may not be enough for
all discoverers to register, which will make test flaky.
This commit changes the waiting logic to wait for number of discoverers
to stop increasing during given time frame, which should be large enough
for single discoverer to register in test environment.
A following run passes with this commit:
go test -failfast -race -count 100 -v ./discovery/kubernetes/
Signed-off-by: Mateusz Gozdek <mgozdekof@gmail.com>
* Add a feature flag to enable the new manager
This PR creates a copy of the legacy manager and uses it by default.
It is a companion PR to #9349. With this PR, users can enable the new
discovery manager and provide us with any feedback / side effects that
the new behaviour might have.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
We are re-enabling HTTP 2 again. There has been a few bugfixes upstream
in go, and we have also enabled ReadIdleTimeout.
Fix#7588Fix#9068
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
We have been Puppet user for 10 years and we are users of
https://github.com/camptocamp/prometheus-puppetdb-sd
However, that file_sd implementation contains business logic and
assumptions around e.g. the modules which you are using.
This pull request adds a simple PuppetDB service discovery, which will
enable more use cases than the upstream sd.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
This change sets the scheme to https when a rule specified by Ingress
matches a wildcard DNS entry in the ingress TLS hosts
Signed-off-by: Philip Gough <philip.p.gough@gmail.com>
* PromQL: Fix start and end keywords masking label and metric names
This commit fixes an issue with the "at modifier" that introduced two
new keywords: `start` and `end`. In grouping options and in metric
names, these keywords took precedence over metric or label names, so
that those metrics and labels could no longer be referenced.
Signed-off-by: Clayton Peters <clayton.peters@man.com>
* Add in additional tests for metrics and/or labels called start/end.
Signed-off-by: Clayton Peters <clayton.peters@man.com>
* *: Cut 2.29.0-rc.0
Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com>
* VERSION: bump to 2.29.0-rc.0
Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com>
* Remove experimental wording on size-based retention
Followup of #9004
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* Fix PR reference in changelog
Signed-off-by: George Brighton <george@gebn.co.uk>
* Describe EC2 availability zone IDs at most once per refresh (#9142)
Signed-off-by: George Brighton <george@gebn.co.uk>
* Describe EC2 availability zones at most once per SD load
Closes#9142.
Signed-off-by: George Brighton <george@gebn.co.uk>
* Incorporate feedback
Signed-off-by: George Brighton <george@gebn.co.uk>
* Integrate feedback
Signed-off-by: George Brighton <george@gebn.co.uk>
* Add a compatibility note for macOS users.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* *: Cut v2.29.0-rc.1
Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com>
* Fix `kuma_sd` targetgroup reporting (#9157)
* Bundle all xDS targets into a single group
Signed-off-by: austin ce <austin.cawley@gmail.com>
* *: cut v2.29.0-rc.2
Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com>
* Rename links
Signed-off-by: Levi Harrison <git@leviharrison.dev>
* bump codemirror-promql to 0.17.0
Signed-off-by: Augustin Husson <husson.augustin@gmail.com>
* *: cut v2.29.0
Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com>
* tsdb: align atomically accessed int64 (#9192)
This prevents a panic in 32-bit archs:
https://pkg.go.dev/sync/atomic#pkg-note-BUGFixed#9190
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
* Release 2.29.1 (#9193)
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
Co-authored-by: Clayton Peters <clayton.peters@man.com>
Co-authored-by: Frederic Branczyk <fbranczyk@gmail.com>
Co-authored-by: George Brighton <george@gebn.co.uk>
Co-authored-by: Austin Cawley-Edwards <austin.cawley@gmail.com>
Co-authored-by: Levi Harrison <git@leviharrison.dev>
Co-authored-by: Augustin Husson <husson.augustin@gmail.com>
* optimize Linode SD by polling for event changes during refresh
Most accounts are fairly "static", in the sense that they're not cycling
through instances constantly. So rather than do a full refresh every
interval and potentially make several behind-the-scenes paginated API
calls, this will now poll the `/account/events/` endpoint every minute
with a list of events that we care about. If a matching event is found,
we then do a full refresh.
Co-authored-by: William Smith <wsmith@linode.com>
Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>
Signed-off-by: William Smith <wsmith@linode.com>
* Fix: Use json.Unmarshal() instead of json.Decoder
See https://ahmet.im/blog/golang-json-decoder-pitfalls/
json.Decoder is for JSON streams, not single JSON objects / bodies.
Signed-off-by: Julius Volz <julius.volz@gmail.com>
* Revert modifications to targetgroup parsing
Signed-off-by: Julius Volz <julius.volz@gmail.com>
prometheus_sd_discovered_targets is wrongly calculated when there are
multiple SD configurations in place. One discovery manager can have
multiple groups coming from multiple service discoveries.
When multiple service discovery configs are used, we do not compute the
metric correctly, and instead just set the metric to one of the service
discoveries.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
This makes it clear that the dockerswarm package does more than docker
swarm, but does also docker.
I have picked moby as it is the upstream name: https://mobyproject.org/
There is no user-facing change, except in the case of a bad
configuration. Previously, a user who would have a bad docker sd config
would see an error like:
> field xx not found in type dockerswarm.plain
Now that error would be turned into:
> field xx not found in type moby.plain
While not perfect, it should at not be confusing between docker and
dockerswarm.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
Prometheus adds the ability to read secrets from files. This add
this feature for the scaleway service discovery.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
This PR introduces support for follow_redirect, to enable users to
disable following HTTP redirects.
Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
The label `__meta_digitalocean_image` expose the `slug` of the image and
the `slug` is only present in the public images.
To refer a user-generated image (`snapshot` or `custom`) we can use
the image's display name.
See: https://developers.digitalocean.com/documentation/v2/#images
Signed-off-by: Matteo Valentini <matteo.valentini@nethesis.it>
Last change in 4efca5a introduced a problem where NewDiscovery would
just return a nil value, which is not handled well and didn't allow for
fixing configuration issues at runtime without a reload.
Signed-off-by: Alfred Krohmer <alfred.krohmer@logmein.com>
This also caches credentials that are obtained e.g. via IRSA on AWS EKS.
Previously, every refresh cycle would request the credentials again.
Signed-off-by: Alfred Krohmer <alfred.krohmer@logmein.com>
Label selector can be
"set-based"(https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#set-based-requirement)
but such a selector causes Prometheus start failure with the "unexpected
error: parsing YAML file ...: invalid selector: 'foo in (bar,baz)';
can't understand 'baz)'"-like error.
This is caused by the `fields.ParseSelector(string)` function that
simply splits an expression as a CSV-list, so a comma confuses such a
parsing method and lead to the error.
Use `labels.Parse(string)` to use a valid lexer to parse a selector
expression.
Closes#8284.
Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
cui fliter