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: bump prometheus version to v0.43 #13159

Closed

Conversation

LukeWinikates
Copy link
Contributor

This PR updates the prometheus package.

The version number looks like a downgrade, but that is because Prometheus has adopted 0. versions for the prometheus package: https://github.com/prometheus/prometheus#prometheus-code-base

Bumping the Prometheus version results in a number of other module versions being updated as well.

It appears to me that the only telegraf code that relies on the prometheus package is the promtheeusremotewrite plugin. I've never looked closely at that plugin, but it seems to pull in the prometheus package specifically for protobuf definitions, and those are available as a separate go package. I'm interested in creating a new GH issue to change that, since using prometheus/prometheus as a library doesn't seem like the recommended approach.

I was surprised to see that the prometheus version used by Telegraf was rather old, and I expected to discover a reason why Telegraf was sticking with an old version. I didn't see any currently open GitHub issues. Hopefully this bump is actually an easy, painless one.

@powersj
Copy link
Contributor

powersj commented May 1, 2023

Hi,

If I am reading through this issue correctly, this PR would bump us from the 1.x to 2.x releases?

Are you planning to circle back around on the lint and build failures?

@powersj powersj added the waiting for response waiting for response from contributor label May 1, 2023
@LukeWinikates LukeWinikates force-pushed the chore-bump-prometheus branch 2 times, most recently from 3e4cf76 to 62353a0 Compare May 1, 2023 23:00
@LukeWinikates
Copy link
Contributor Author

Thanks @powersj

If I am reading through prometheus/prometheus#8852 correctly, this PR would bump us from the 1.x to 2.x releases?

Yes, that's how I understand it.

I think that because this prometheus version is quite old, its version constraints have prevented several other plugins' dependencies from being updated. I would say that means that updating prometheus is a good thing, but it also means that updating prometheus updates a number of other dependencies, changing some small apis and producing new linter errors.

  1. make check-deps produces this:
2
+++ /var/folders/m6/331kxszx1q70n8_z62ldqrfr0000gn/T/tmp.n97vfT95/HEAD	2023-05-01 15:58:42
@@ -41 +40,0 @@
-github.com/apache/arrow/go
@@ -42,0 +42 @@
+github.com/apache/arrow/go

I think that means that arrow is an edge case that the script file doesn't handle well for some reason. I'll try to figure out what to do about it, but I'll have to take a closer look at that bash script and possibly make some changes.

request: I'd love a hint from somebody who knows this script better than I do.

  1. GOARCH=386 make check prints some warnings about arrow. I'm not sure what this make target is meant to catch.

question: Is this a legitimate failure or should it be ignored?

@telegraf-tiger telegraf-tiger bot removed the waiting for response waiting for response from contributor label May 1, 2023
@srebhan
Copy link
Contributor

srebhan commented May 2, 2023

Hey @LukeWinikates, the full output of make check-deps is

@@ -4 +3,0 @@
-github.com/99designs/go-keychain
@@ -41 +39,0 @@
-github.com/apache/arrow/go
@@ -42,0 +41 @@
+github.com/apache/arrow/go

which means that github.com/99designs/go-keychain should not be listed as a dependency. and that github.com/apache/arrow/go is in the wrong place (i.e. it should be one line down) as we enforce alphabetical ordering.

@srebhan srebhan self-assigned this May 2, 2023
@srebhan srebhan added the dependencies Pull requests that update a dependency file label May 2, 2023
Copy link
Contributor

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

LGTM once the dependencies licence-document is fixed.

@powersj
Copy link
Contributor

powersj commented May 2, 2023

@LukeWinikates I pushed the changes + rebase to resolve conflicts

@srebhan this is the second time I am seeing this now, this time with apache arrow v10:

go vet $(go list ./... | grep -v ./plugins/parsers/influx)
# github.com/apache/arrow/go/v10/internal/utils
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:76:4: undefined: TransposeInt8Int8
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:78:4: undefined: TransposeInt8Int16
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:80:4: undefined: TransposeInt8Int32
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:82:4: undefined: TransposeInt8Int64
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:84:4: undefined: TransposeInt8Uint8
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:86:4: undefined: TransposeInt8Uint16
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:88:4: undefined: TransposeInt8Uint32
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:90:4: undefined: TransposeInt8Uint64
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:95:4: undefined: TransposeInt16Int8
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:97:4: undefined: TransposeInt16Int16
/go/pkg/mod/github.com/apache/arrow/go/v10@v10.0.1/internal/utils/transpose_ints_def.go:97:4: too many errors

@srebhan
Copy link
Contributor

srebhan commented May 2, 2023

Yeah the other one is #13170 with arrow v11. The issue is fixed in v12 but the dependencies (for #13170 it's bigquery) have not yet adapted to v12... @powersj can you do a go mod why github.com/apache/arrow/go/v10/internal/utils?

@powersj
Copy link
Contributor

powersj commented May 2, 2023

❯ go mod why github.com/apache/arrow/go/v10/internal/utils
# github.com/apache/arrow/go/v10/internal/utils
github.com/influxdata/telegraf/plugins/outputs/bigquery
cloud.google.com/go/bigquery
github.com/apache/arrow/go/v10/arrow/array
github.com/apache/arrow/go/v10/internal/utils

@srebhan
Copy link
Contributor

srebhan commented May 2, 2023

Then we have to wait for the upstream issue (see my comment in #13170) to be resolved and bump bigquery first...

@LukeWinikates
Copy link
Contributor Author

Thanks @powersj and @srebhan

Given the issue with arrow, I think there are two options for bumping the prometheus dependency.

1: wait for bigquery to release a new version that depends on arrow v12
2: instead of targeting promtheus v0.43, it looks like we could target v0.42, which is compatible with telegraf's current version ofbigquery, v1.45.0

It looks like the fix for bigquery might land pretty soon, so waiting for that might make sense. But since prometheus in telegraf is pretty far behind the upstream, and there are a fair number of changes in this PR, I do kind of like the idea of just upgrading prometheus to a recent version right away.

Curious what you think - I'd be happy to update this PR or open a new one targeting prometheus v0.42

@LukeWinikates LukeWinikates changed the title chore: bump prometheus version chore: bump prometheus version to v0.43 May 2, 2023
@powersj
Copy link
Contributor

powersj commented May 2, 2023

Curious what you think - I'd be happy to update this PR or open a new one targeting prometheus v0.42

It looked like the upstream PR was getting traction, but I am happy to take a change now so you can continue to work and test this

@LukeWinikates LukeWinikates deleted the chore-bump-prometheus branch May 3, 2023 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants