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

chore: update prometheus dependency for service discovery #2536

Merged
merged 1 commit into from
Apr 14, 2021

Conversation

lesam
Copy link
Contributor

@lesam lesam commented Apr 13, 2021

Closes: #2532

There were significant reformatting changes in the prometheus service discovery
packages and their configuration, but it seems like the logic is mostly similar.

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated

@lesam
Copy link
Contributor Author

lesam commented Apr 13, 2021

Do we want a changelog for this?
probably, it is a pretty big library update.

@lesam lesam requested a review from docmerlin April 13, 2021 20:02
@@ -1,58 +0,0 @@
FROM 32bit/ubuntu:16.04
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Delete the 32 bit tests since we will soon be deleting the 32 bit build.

@lesam lesam force-pushed the prometheus-update branch from a94ebff to 8ccd227 Compare April 13, 2021 20:15
github.com/pierrec/lz4 v2.5.2+incompatible // indirect
github.com/pkg/errors v0.9.1
github.com/prometheus/common v0.20.0
github.com/prometheus/prometheus v1.8.2-0.20210331101223-3cafc58827d1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually tag v2.26.0 of prometheus, but prometheus/prometheus#6048 prevents us from using the semver name, we have to specify the commit instead.

@lesam lesam force-pushed the prometheus-update branch from 8ccd227 to b25ed27 Compare April 13, 2021 20:30
mgr interface {
ApplyConfig(cfg *config.Config) error
Stop()
Start()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The prometheus fork we had was just to split Run into Start and Wait. But the latest version of prometheus looks like it should support calling Stop before Run in racy cases, so I don't think we need the fork any longer.

Copy link
Contributor

Choose a reason for hiding this comment

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

awesome!

@lesam lesam force-pushed the prometheus-update branch 2 times, most recently from c224fa0 to 6e110ea Compare April 13, 2021 21:07
@lesam
Copy link
Contributor Author

lesam commented Apr 14, 2021

@lesam lesam changed the title chore: update prometheus depedency chore: update prometheus dependency for service discovery Apr 14, 2021
Closes: #2532

There were significant reformatting changes in the prometheus service discovery
packages and their configuration, but it seems like the logic is mostly similar.
@lesam lesam force-pushed the prometheus-update branch from 6e110ea to c46b60b Compare April 14, 2021 19:45
@@ -27,11 +28,11 @@ type Config struct {
SSLServerName string `toml:"ssl-server-name" override:"ssl-server-name"`
// Use SSL but skip chain & host verification
InsecureSkipVerify bool `toml:"insecure-skip-verify" override:"insecure-skip-verify"`
// This previously supported Timeout but it was removed, see https://github.com/prometheus/common/pull/123
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs a docs change.

@lesam lesam merged commit 59e3187 into master Apr 14, 2021
@docmerlin docmerlin deleted the prometheus-update branch April 15, 2021 15:20
sranka pushed a commit that referenced this pull request May 1, 2021
Closes: #2532

There were significant reformatting changes in the prometheus service discovery
packages and their configuration, but it seems like the logic is mostly similar.
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.

Update prometheus dependency
2 participants