Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

Moves metrics registration at the end of initialization #340

Merged
merged 1 commit into from
Oct 21, 2022

Conversation

cyriltovena
Copy link
Collaborator

This is a bandaid to avoid having to refactor everything because 0.1 but ultimately we should be refactoring our metric registration.

Copy link
Collaborator

@simonswine simonswine left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +116 to +120
reg := phlarecontext.Registry(phlarectx)

// ensure head metrics are registered early so they are reused for the new head
phlarectx = contextWithHeadMetrics(phlarectx, newHeadMetrics(reg))
f.phlarectx = phlarectx
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe that is clearer:

Suggested change
reg := phlarecontext.Registry(phlarectx)
// ensure head metrics are registered early so they are reused for the new head
phlarectx = contextWithHeadMetrics(phlarectx, newHeadMetrics(reg))
f.phlarectx = phlarectx
// ensure head metrics are registered early so they are reused for the new head
f.phlarectx = contextWithHeadMetrics(phlarectx, newHeadMetrics(phlarecontext.Registry(phlarectx))

@cyriltovena cyriltovena merged commit 09b12a5 into main Oct 21, 2022
@cyriltovena cyriltovena deleted the panic-metrics branch October 21, 2022 09:23
simonswine pushed a commit to simonswine/pyroscope that referenced this pull request Jun 30, 2023
Moves metrics registration at the end of initialization
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants