Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace node_exporter https package with exporter-toolkit #230

Merged
merged 3 commits into from
Dec 22, 2021

Conversation

rfratto
Copy link
Contributor

@rfratto rfratto commented Oct 22, 2021

The node_exporter/https package has been moved to exporter-toolkit as of v1.1.0. weaveworks/common was still using this package, which was preventing downstream importers (i.e., grafana/agent) from updating their dependency on node_exporter.

This change also required bumping kuberesolver to v2.4.0 (the next version after v2.1.0) which uses a newer version of the gRPC library where type names have changed. The kuberesolver diff between the two versions can be found here: sercand/kuberesolver@v2.1.0...v2.4.0

The node_exporter/https package has been moved to exporter-toolkit as of
v1.1.0. weaveworks/common was still using this package, which was
preventing downstream importers (i.e., grafana/agent) from updating
their dependency on node_exporter.

This change also required bumping kuberesolver to v2.4.0 (the next
version after v2.1.0) which uses a newer version of the gRPC library
where type names have changed.

kuberesolver diff: sercand/kuberesolver@v2.1.0...v2.4.0
tcp_connections{protocol="http"} 0
tcp_connections{protocol="grpc"} 0
`), "request_message_bytes", "response_message_bytes", "inflight_requests", "tcp_connections"))
# HELP inflight_requests Current number of inflight requests.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eugh, sorry, gopls format did this. Should I revert?

@rfratto
Copy link
Contributor Author

rfratto commented Oct 22, 2021

🤔 Looking into lint/test errors. This had worked on my machine just fine, which makes me feel like I probably have a newer Go version which is being helpful.

prometheus/exporter-toolkit requires 1.14
@rfratto
Copy link
Contributor Author

rfratto commented Oct 22, 2021

🤔 Looking into lint/test errors. This had worked on my machine just fine, which makes me feel like I probably have a newer Go version which is being helpful.

Oh 😞 exporter-toolkit requires at least Go 1.14. I bumped the versions here, but I'm concerned about how much that bloats up this PR.

@bboreham
Copy link
Collaborator

bboreham commented Nov 4, 2021

The kuberesolver diff between the two versions can be found here: sercand/kuberesolver@v2.1.0...v2.4.0

Do you think you could list the main points in English rather than asking everyone to read the diffs?

Not too bothered about the whitespace changes, as long as they are called out in the commit message.
It would be better to remove them from this PR and do a separate update to the whole library to current formatting standards.

Updating to Go 1.14 is fine; kuberesolver and gRPC are the main things which catch my eye as potentially affecting systems.

@rfratto
Copy link
Contributor Author

rfratto commented Nov 4, 2021

Do you think you could list the main points in English rather than asking everyone to read the diffs?

@bboreham Absolutely.

  • Kuberesolver now defines two global metrics for tracking endpoints and addresses.
  • Types have been updated to the breaking changes in the type names introduced by gRPC v1.27

gRPC has changed more dramatically between v1.26 and v1.31. I'll link to their releases which each have changelogs:

(For reference, the latest Prometheus release is using v1.40.0, if that means that this module should too)

Copy link
Collaborator

@bboreham bboreham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants