Several things done here:
- Set `max-issues-per-linter` to 0 so that we actually see all linter
warnings and not just 50 per linter. (As we also set
`max-same-issues` to 0, I assume this was the intention from the
beginning.)
- Stop using the golangci-lint default excludes (by setting
`exclude-use-default: false`. Those are too generous and don't match
our style conventions. (I have re-added some of the excludes
explicitly in this commit. See below.)
- Re-add the `errcheck` exclusion we have used so far via the
defaults.
- Exclude the signature requirement `govet` has for `Seek` methods
because we use non-standard `Seek` methods a lot. (But we keep other
requirements, while the default excludes completely disabled the
check for common method segnatures.)
- Exclude warnings about missing doc comments on exported symbols. (We
used to be pretty adamant about doc comments, but stopped that at
some point in the past. By now, we have about 500 missing doc
comments. We may consider reintroducing this check, but that's
outside of the scope of this commit. The default excludes of
golangci-lint essentially ignore doc comments completely.)
- By stop using the default excludes, we now get warnings back on
malformed doc comments. That's the most impactful change in this
commit. It does not enforce doc comments (again), but _if_ there is
a doc comment, it has to have the recommended form. (Most of the
changes in this commit are fixing this form.)
- Improve wording/spelling of some comments in .golangci.yml, and
remove an outdated comment.
- Leave `package-comments` inactive, but add a TODO asking if we
should change that.
- Add a new sub-linter `comment-spacings` (and fix corresponding
comments), which avoids missing spaces after the leading `//`.
Signed-off-by: beorn7 <beorn@grafana.com>
This commit removes support for the following API versions:
* `discovery.k8s.io/v1beta1` API version of EndpointSlice (no longer
served as of v1.25).
* `networking.k8s.io/v1beta1` API version of Ingress (no longer served
as of v1.22).
Closes#12884
Signed-off-by: Simon Pasquier <spasquie@redhat.com>
* feat: add support for gathering flavor name in Openstack discovery
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
* Update instance.go
Co-authored-by: Ayoub Mrini <ayoubmrini424@gmail.com>
Signed-off-by: Paulo Dias <44772900+paulojmdias@users.noreply.github.com>
* Update configuration.md
Co-authored-by: Ayoub Mrini <ayoubmrini424@gmail.com>
Signed-off-by: Paulo Dias <44772900+paulojmdias@users.noreply.github.com>
* fix: fix linting
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
* fix: fix instance type
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
* Update docs/configuration/configuration.md
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Signed-off-by: Paulo Dias <44772900+paulojmdias@users.noreply.github.com>
---------
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
Signed-off-by: Paulo Dias <44772900+paulojmdias@users.noreply.github.com>
Co-authored-by: Ayoub Mrini <ayoubmrini424@gmail.com>
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
[ENHANCEMENT] Docker SD: add MatchFirstNetwork for containers with multiple networks
Fixes docker sd service misssing in shared mode and deduplicate targets by network
* discovery: aws: expose Primary IPv6 addresses as label
Add __meta_ec2_primary_ipv6_addresses label. This label contains the
Primary IPv6 address for every ENI attached to the EC2 instance. It is
ordered by the DeviceIndex and the missing elements (interface without
Primary IPv6 address) are kept in the list.
---------
Signed-off-by: Arpad Kunszt <akunszt@hiya.com>
Co-authored-by: Ayoub Mrini <ayoubmrini424@gmail.com>
to show "targets groups update" starvation when the notifications queue is full and an Alertmanager
is down.
The existing `TestHangingNotifier` that was added in https://github.com/prometheus/prometheus/pull/10948 doesn't really reflect the reality as the SD changes are manually fed into `syncCh` in a continuous way, whereas in reality, updates are only resent every `updatert`.
The test added here sets up an SD manager and links it to the notifier. The SD changes will be triggered by that manager as it's done in reality.
Signed-off-by: machine424 <ayoubmrini424@gmail.com>
Co-authored-by: Ethan Hunter <ehunter@hudson-trading.com>
Ran "make update-all-go-deps" , but do not bump go version and remove
toolchain clause from go.mod
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
* fix(scaleway-sd): use public IPs if no private IP present
* tests(scaleway-sd): add instance with routed public ip and no private ip
---------
Signed-off-by: Heyoxe <32708033+Heyoxe@users.noreply.github.com>
Previously the metrics adapters for client-go were registered in an init function.
This resulted in clobbering default metrics providers when these packages are imported
into an application that leverages the default client-go metrics registry.
Instead, let's only register these adapters when requested.
Signed-off-by: Stephen Heckler <sheckler@cloudflare.com>
This commit adds 2 new metadata labels for the endpointslice role:
* `__meta_kubernetes_endpointslice_endpoint_node_name`
* `__meta_kubernetes_endpointslice_endpoint_zone`
The latter is only present when the `discovery.k8s.io/v1` API group is
available.
I also updated the configuration doc and added an entry for the
`__meta_kubernetes_endpointslice_endpoint_hostname` label which was
missing.
Signed-off-by: Simon Pasquier <spasquie@redhat.com>
* Add support for discovering Linode IPv6 ranges associated with linodes.
* Add optional but recommended region filtering (faster queries, more relevant information).
* Added missing fields in configuration.md, updated linode test cases.
* Convert to TableDrivenTests as per tjhop request.
Signed-off-by: David Andruczyk <dandrucz@akamai.com>
This adds support for Consul's Catalog [List Services][^1] API's `filter`
parameter added in 1.14.x. This parameter grants the operator more
flexibility to do server-side filtering of the Catalog, before
Prometheus subscribes for updates. Operators can use this to improve
both the performance of Prometheus's Consul SD and reduce the impact of
enumerating large catalogs.
[^1]: https://developer.hashicorp.com/consul/api-docs/v1.14.x/catalog
Signed-off-by: Daniel Kimsey <dekimsey@protonmail.com>
* fix(discovery/kubernetes/endpoints): react to changes on Pods because some modifications can occur on them without triggering an update on the related Endpoints (The Pod phase changing from Pending to Running e.g.).
---------
Signed-off-by: machine424 <ayoubmrini424@gmail.com>
Co-authored-by: Guillermo Sanchez Gavier <gsanchez@newrelic.com>
SD Managers take over responsibility for SD metrics registration
---------
Signed-off-by: Paulin Todev <paulin.todev@gmail.com>
Signed-off-by: Björn Rabenstein <github@rabenste.in>
Co-authored-by: Björn Rabenstein <github@rabenste.in>
The loop ran indefinitely if the condition isn't met.
Before, each iteration created a new timer channel which was always outpaced by
the other timer channel with smaller duration.
minor detail: There was a memory leak: resources of the ~10 previous timers were
constantly kept. With the fix, we may keep the resources of one timer around for defaultWait
but this isn't worth the changes to make it right.
Signed-off-by: machine424 <ayoubmrini424@gmail.com>
Calling `*armnetwork.InterfacesClient.Get()` doesn't work for Scale Set
VM NIC, because these use a different Resource ID format.
Use `*armnetwork.InterfacesClient.GetVirtualMachineScaleSetNetworkInterface()`
instead. This needs both the scale set name and the instance ID, so
add an `InstanceID` field to the `virtualMachine` struct. `InstanceID`
is empty for a VM that isn't a ScaleSetVM.
Signed-off-by: Daniel Nicholls <daniel.nicholls@resdiary.com>
Calling `*armnetwork.InterfacesClient.Get()` doesn't work for Scale Set
VM NIC, because these use a different Resource ID format.
Use `*armnetwork.InterfacesClient.GetVirtualMachineScaleSetNetworkInterface()`
instead. This needs both the scale set name and the instance ID, so
add an `InstanceID` field to the `virtualMachine` struct. `InstanceID`
is empty for a VM that isn't a ScaleSetVM.
Signed-off-by: Daniel Nicholls <daniel.nicholls@resdiary.com>
Version 2 introduced a breaking change in the `id` field of all
resources. They were changed from `int` to `int64` to make sure that all
future numerical IDs are supported on all architectures.
You can learn more about this
[here](https://docs.hetzner.cloud/#deprecation-notices-%E2%9A%A0%EF%B8%8F)
Signed-off-by: Julian Tölle <julian.toelle@hetzner-cloud.de>
InstanceSpec struct members are untyped integers, so they can overflow
on 32-bit arch when bit-shifted left.
Signed-off-by: Daniel Swarbrick <daniel.swarbrick@gmail.com>
Adds a new label to include the ID of the image that an instance is
using. This can be used for example to filter a job to only include
instances using a certain image as that image includes some exporter.
Sometimes the image information isn't available, such as when the image
is private and the user doesn't have the roles required to see it. In
those cases we just don't set the label, as the rest of the information
from the discovery provider can still be used.
Signed-off-by: Taavi Väänänen <hi@taavi.wtf>
Previously, `d.paths` were normalized to backslashes on Windows, even when if
the config file used Unix style. The end result meant always watching `./`, so
changes for this config were always ignored:
scrape_configs:
- job_name: 'envmsc1'
file_sd_configs:
- files:
- 'targets/envmsc1.d/*.yml'
- 'targets/envmsc1.d/*.yaml'
Additionally, unlike the other platforms, no warning was emitted on startup
about not being able to install the watch if the directory didn't exist. Now it
is logged.
Signed-off-by: Matt Harbison <mharbison@atto.com>
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>
machine424
Bryan Boreham
beorn7
Jan Fajerski
ouyang1204@gmail.com
Arve Knudsen
Julien Pivotto
cuiweiyuan
Julien
Simon Pasquier
Paulo Dias
akunszt
Jules Casteran
György Krajcsovits
Arthur Silva Sens
David Andruczyk
Oleksandr Redko
Heyoxe
Stephen Heckler
Matthieu MOREL
hanghuge
David Ashpole
dandrucz
sellskin
Daniel Kimsey
Jan-Otto Kröpke
michaelact
Augustin Husson
Darshan Chaudhary
Paweł Szulik
tyltr
Paulin Todev
Bartlomiej Plotka
daniel-resdiary
Daniel Nicholls
Marcin Skalski
Gavin Lam
Daniel Jolly
Oleksandr Redko
Charles Korn
Etourneau Gwenn
Jayapriya Pai
Marc Tuduri
Julian Tölle
Daniel Swarbrick
Mikhail Fesenko
haleyao
Taavi Väänänen
Matt Harbison
renzheng.wang
Mickael Carl
cui fliter
Fish-pro
Peter Nicholson
Frederic Branczyk
Ben Whetstone
Jens Erat
David Fridman
Aaron George
Levi Harrison
Michael Fuller
Marine Bal
Viacheslav Panasovets
David Cañadillas