Skip to content

Rework SLO metrics - #2840

Merged
joe-elliott merged 14 commits into
grafana:mainfrom
joe-elliott:slo-fighting
Aug 25, 2023
Merged

Rework SLO metrics#2840
joe-elliott merged 14 commits into
grafana:mainfrom
joe-elliott:slo-fighting

Conversation

@joe-elliott

@joe-elliott joe-elliott commented Aug 24, 2023

Copy link
Copy Markdown
Collaborator

What this PR does:
This PR reworks the way our SLO metrics are recorded with these goals in mind;

  • Push the calculations out to be as close the http.Server as possible
  • Clearly metric both total queries and SLO queries in one spot
  • Cleanup and remove unused metrics

This PR accomplishes the above with the following sharp corners:

  • In order to pass the throughput up to the code that calculates the SLOs a *float was added to the context. This works, but is brittle and difficult to detect if it breaks.
  • Specialized code needed to be added to the streaming endpoint

TODO

  • correct changelog
  • add tests

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
@joe-elliott joe-elliott changed the title Remove status from total_queries Aug 24, 2023
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>

@zalegrala zalegrala left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks reasonable to me. Just a question about tracking the other operations. Have you had a chance to test this out?

Comment thread modules/frontend/slos.go
Signed-off-by: Joe Elliott <number101010@gmail.com>
Signed-off-by: Joe Elliott <number101010@gmail.com>

@mdisibio mdisibio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, accuracy of the metrics for SLO calculation is important and I think this is a good change. I think this is Tempo's first case of context smuggling so it gets a pass. 😄

Signed-off-by: Joe Elliott <number101010@gmail.com>

@zalegrala zalegrala left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work Joe. Thanks for giving this area some ❤️ .

Signed-off-by: Joe Elliott <number101010@gmail.com>
@joe-elliott
joe-elliott merged commit f5c8e78 into grafana:main Aug 25, 2023
mattdurham pushed a commit to mattdurham/tempo that referenced this pull request Jun 18, 2026
* better align total queries

Signed-off-by: Joe Elliott <number101010@gmail.com>

* comments

Signed-off-by: Joe Elliott <number101010@gmail.com>

* changelog

Signed-off-by: Joe Elliott <number101010@gmail.com>

* first pass new slo structure

Signed-off-by: Joe Elliott <number101010@gmail.com>

* added slos.go

Signed-off-by: Joe Elliott <number101010@gmail.com>

* pass throughput correctly

Signed-off-by: Joe Elliott <number101010@gmail.com>

* Added SLOs to streaming

Signed-off-by: Joe Elliott <number101010@gmail.com>

* correct metrics order

Signed-off-by: Joe Elliott <number101010@gmail.com>

* added tests. fixed calcs

Signed-off-by: Joe Elliott <number101010@gmail.com>

* changelog

Signed-off-by: Joe Elliott <number101010@gmail.com>

* safety check and postSLOHook

Signed-off-by: Joe Elliott <number101010@gmail.com>

* lint

Signed-off-by: Joe Elliott <number101010@gmail.com>

* lint

Signed-off-by: Joe Elliott <number101010@gmail.com>

* e2e

Signed-off-by: Joe Elliott <number101010@gmail.com>

---------

Signed-off-by: Joe Elliott <number101010@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants