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

Move ingester_traces_created_total #2884

Merged
merged 2 commits into from
Sep 1, 2023

Conversation

joe-elliott
Copy link
Member

@joe-elliott joe-elliott commented Sep 1, 2023

What this PR does:
Moves the location we are calculating tempo_ingester_traces_created_total to the point at which a trace is appended to the wal block. This will allow for better alignment with tempodb_warnings_total{reason="disconnected_trace_flushed_to_wal"} which improves the accuracy of the formulas recommended here:

#2790

Preferring this to adding a new multitenant metric. This change doesn't really lower the value of tempo_ingester_traces_created_total it just subtly changes it's meaning. Given that it still broadly metrics traces created total this feels fine to me, but willing to be disagreed with.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Copy link
Member

@mapno mapno left a comment

Choose a reason for hiding this comment

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

LGTM

@joe-elliott joe-elliott merged commit 25dc436 into grafana:main Sep 1, 2023
galalen pushed a commit to galalen/tempo that referenced this pull request Sep 3, 2023
* move metric

Signed-off-by: Joe Elliott <[email protected]>

* changelog

Signed-off-by: Joe Elliott <[email protected]>

---------

Signed-off-by: Joe Elliott <[email protected]>
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.

2 participants