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

Update to Prometheus v2.54.1 #1574

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Update to Prometheus v2.54.1 #1574

wants to merge 4 commits into from

Conversation

ptodev
Copy link
Collaborator

@ptodev ptodev commented Aug 28, 2024

PR Description

Upgrading to the latest Prometheus version - v0.54.1.

I didn't add bugfixes to the changelog, because I'm not too sure which ones exactly affect Alloy.

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

This comment was marked as outdated.

@ptodev ptodev force-pushed the ptodev/upgrade-prometheus branch 2 times, most recently from 61de1dc to 5549f2a Compare October 7, 2024 17:11
@ptodev ptodev changed the title Update to Prometheus v0.54.1 Update to Prometheus v2.54.1 Oct 7, 2024
@ptodev ptodev marked this pull request as ready for review October 7, 2024 17:11
@ptodev ptodev requested review from clayton-cornell and a team as code owners October 7, 2024 17:11
@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Oct 7, 2024
@ptodev ptodev force-pushed the ptodev/upgrade-prometheus branch from 5549f2a to be17248 Compare February 3, 2025 12:38
Copy link
Contributor

github-actions bot commented Feb 3, 2025

@ptodev ptodev force-pushed the ptodev/upgrade-prometheus branch 2 times, most recently from 933b511 to 2558a85 Compare February 5, 2025 18:14
@ptodev ptodev force-pushed the ptodev/upgrade-prometheus branch 4 times, most recently from 551a3f6 to 5e9daee Compare February 6, 2025 16:22
@ptodev ptodev force-pushed the ptodev/upgrade-prometheus branch from 5e9daee to 2818d1e Compare February 6, 2025 16:56
@@ -69,6 +69,8 @@ type Arguments struct {
Params url.Values `alloy:"params,attr,optional"`
// Whether to scrape a classic histogram that is also exposed as a native histogram.
ScrapeClassicHistograms bool `alloy:"scrape_classic_histograms,attr,optional"`
// Whether to scrape native histograms.
ScrapeNativeHistograms bool `alloy:"scrape_native_histograms,attr,optional"`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Related to this Prometheus change: prometheus/prometheus#13987

We don't have to have the new scrape_native_histograms argument in order to maintain backwards compatibility with older Alloy versions. However, the Prometheus docs mention that much of this functionality is experimental, so I thought it'd be good to be able to disable it.

By default scrape_native_histograms is set to true to maintain backwards compatibility.

Duration: s.metrics.requestDuration,
RequestBodySize: s.metrics.rxMessageSize,
ResponseBodySize: s.metrics.txMessageSize,
InflightRequests: s.metrics.inflightRequests,
}

ri := middleware.RouteInjector{
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this? Is this part of the upgrade?

@@ -242,6 +243,9 @@ func convertConfigs(cfg Arguments) (*config.Config, error) {
SendExemplars: rw.SendExemplars,
SendNativeHistograms: rw.SendNativeHistograms,

//TODO: Make this configurable?
ProtobufMessage: config.RemoteWriteProtoMsgV1,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels like we would need this when we upgrade to prom 3.0, so maybe add that to the todo?

}

var err error
_, err = uuid.Parse(a.OAuth.ClientID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this specific to Azure? Not sure in generic oauth this is true.

Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

A few small clarifications but overall LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants