Skip to content

Conversation

joshhunt
Copy link
Contributor

@joshhunt joshhunt commented Sep 1, 2025

  • Adds a http_response_size_bytes histogram metric to track response sizes.
  • Breaks the /public/ route handler label value down into more granular plugin-assets and public-build-assets. public-assets is still there, but is a fallback that we want to get down to 0.

Fixes https://github.com/grafana/grafana-frontend-platform/issues/33

@joshhunt joshhunt marked this pull request as ready for review September 2, 2025 09:59
@joshhunt joshhunt requested review from a team as code owners September 2, 2025 09:59
@joshhunt joshhunt requested review from samsch and ashharrison90 and removed request for a team September 2, 2025 09:59
@github-actions github-actions bot added this to the 12.2.x milestone Sep 2, 2025
Copy link
Contributor

@ashharrison90 ashharrison90 left a comment

Choose a reason for hiding this comment

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

lgtm 👍

func RequestMetrics(features featuremgmt.FeatureToggles, cfg *setting.Cfg, promRegister prometheus.Registerer) web.Middleware {
log := log.New("middleware.request-metrics")

log.Info("initializing request metrics middleware", "size_buckets", sizeDefBuckets)
Copy link
Contributor

Choose a reason for hiding this comment

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

was this for debug, or do we actually want it? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope!!!! thanks!

@joshhunt joshhunt force-pushed the jh/response-size-metric branch from 21c1fa8 to 4168e61 Compare September 2, 2025 10:05
durationHistogram.Observe(elapsedTime)
}

sizeHistogram.Observe(float64(rw.Size()))
Copy link
Member

Choose a reason for hiding this comment

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

why not also observe with an exemplar for this one? seems like it might be useful...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ngl the code kinda scared me a little bit so i just ignored it, figuring if it's the right thing to do someone will tell me!

Copy link
Member

@hairyhenderson hairyhenderson left a comment

Choose a reason for hiding this comment

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

:shipit:

@joshhunt joshhunt merged commit bda895e into main Sep 3, 2025
130 checks passed
@joshhunt joshhunt deleted the jh/response-size-metric branch September 3, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants