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

Upgrade prometheus/client_golang and reconfigure to restore go_sched.* metrics #6957

Merged

Conversation

dannykopping
Copy link
Contributor

@dannykopping dannykopping commented Aug 23, 2022

What this PR does / why we need it:
These metrics were removed unintentionally in #6403.

The go_sched_latencies histogram is very important for determining pathological scheduler behaviour (I need it to verify the impact of #6954).

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:
Don't be afraid of the PR size; it's mainly just revendored code.

Checklist

  • Documentation added
  • Tests updated
  • Is this an important fix or new feature? Add an entry in the CHANGELOG.md.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

These metrics were removed unintentionally in grafana#6403

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
@dannykopping dannykopping force-pushed the dannykopping/restore-go-sched-metrics branch from 9320d17 to 47ba154 Compare August 23, 2022 15:12
Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0.1%

@dannykopping dannykopping changed the title Upgrading prometheus/client_golang to restore go_sched.* metrics Upgrade prometheus/client_golang to restore go_sched.* metrics Aug 23, 2022
@dannykopping dannykopping changed the title Upgrade prometheus/client_golang to restore go_sched.* metrics Upgrade prometheus/client_golang and reconfigure to restore go_sched.* metrics Aug 23, 2022
@dannykopping dannykopping marked this pull request as ready for review August 23, 2022 15:24
@dannykopping dannykopping requested a review from a team as a code owner August 23, 2022 15:24
Copy link
Contributor

@MichelHollands MichelHollands left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -96,8 +97,23 @@ const (
UsageReport string = "usage-report"
)

func (t *Loki) initRegisterer() prometheus.Registerer {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: The name of this function makes it look like there is a module called Registerer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true, I think I'll leave this for now and refactor it when I get rid of the reliance on the prometheus.DefaultXXX references

@dannykopping
Copy link
Contributor Author

@DylanGuedes raised a point in Slack: will the upgrade of prometheus/common from v0.35.0 to v0.37.0 present any challenges?

prometheus/common@v0.35.0...v0.37.0

The diff doesn't seem to show anything too scary, and unfortunately there is no changelog in prometheus/common, so we'll just have to see...

@dannykopping dannykopping merged commit 2321da4 into grafana:main Aug 23, 2022
@dannykopping dannykopping deleted the dannykopping/restore-go-sched-metrics branch August 23, 2022 15:38
dannykopping pushed a commit that referenced this pull request Aug 23, 2022
….*` metrics (#6957)

* Upgrading prometheus/client_golang to restore go_sched.* metrics

These metrics were removed unintentionally in #6403

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>

* Fixing test to match interface correctly

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
dannykopping pushed a commit to dannykopping/loki that referenced this pull request Aug 24, 2022
Adding all available metrics from Go

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
dannykopping pushed a commit that referenced this pull request Aug 24, 2022
Adding all available metrics from Go

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
dannykopping pushed a commit that referenced this pull request Aug 24, 2022
Adding all available metrics from Go

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
@periklis
Copy link
Collaborator

@dannykopping Can we consider backporting this to 2.6? My key pain point is:

[BUGFIX] Raise exemplar labels limit from 64 to 128 bytes as specified in OpenMetrics spec. https://github.com/prometheus/client_golang/pull/1091

@dannykopping dannykopping added the backport release-2.6.x Tag a PR with this label to create a PR which cherry pics it into the release-2.6.x branch label Aug 29, 2022
@dannykopping
Copy link
Contributor Author

Sure 👍

@grafanabot
Copy link
Collaborator

The backport to release-2.6.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-6957-to-release-2.6.x origin/release-2.6.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 2321da49b580dd00204fe9f19a0db837af8d19f0
# Push it to GitHub
git push --set-upstream origin backport-6957-to-release-2.6.x
git switch main
# Remove the local backport branch
git branch -D backport-6957-to-release-2.6.x

Then, create a pull request where the base branch is release-2.6.x and the compare/head branch is backport-6957-to-release-2.6.x.

dannykopping pushed a commit that referenced this pull request Aug 29, 2022
Adding all available metrics from Go

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
lxwzy pushed a commit to lxwzy/loki that referenced this pull request Nov 7, 2022
….*` metrics (grafana#6957)

* Upgrading prometheus/client_golang to restore go_sched.* metrics

These metrics were removed unintentionally in grafana#6403

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>

* Fixing test to match interface correctly

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
lxwzy pushed a commit to lxwzy/loki that referenced this pull request Nov 7, 2022
…grafana#6962)

Adding all available metrics from Go

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-2.6.x Tag a PR with this label to create a PR which cherry pics it into the release-2.6.x branch size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants