Skip to content

Conversation

@simonswine
Copy link
Contributor

@simonswine simonswine commented Jun 13, 2025

This implements the ability to add artificial delay to the ingestion handlers.

This is useful during the v2 migration process in order to spot how client will behave

Inspired by Mimir implementation:

This relies on #4249

Once this is enabled, we also need to update dashboards to show the metrics without artificial delay

@simonswine simonswine changed the title 20250612 artificial delay feat: Implement artificial ingestion delay. Jun 13, 2025
@simonswine simonswine force-pushed the 20250612_artificial-delay branch 2 times, most recently from 215388a to 6cb7d77 Compare June 13, 2025 13:04
@simonswine simonswine marked this pull request as ready for review June 13, 2025 13:25
@simonswine simonswine requested review from a team, aleks-p and marcsanmi as code owners June 13, 2025 13:25
Copy link
Collaborator

@kolesnikovae kolesnikovae left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for adding this!

Comment on lines 64 to 77
func NewHTTP(limits Limits) func(h http.Handler) http.Handler {
return func(h http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
start := timeNow()
ctx := r.Context()

delay := getDelay(ctx, limits)
var delayRw *delayedResponseWriter
if delay > 0 {
w, delayRw = wrapResponseWriter(w, start.Add(delay))
}

// now run the chain after me
h.ServeHTTP(w, r.WithContext(ctx))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure why we replace the request context. If we haven't modified it, it should not be needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spotted, I had a trace span created in some previous version

This implements the ability to add artificial delay to the ingestion handlers.

This is useful during the v2 migration process in order to spot how client will behave

Fixes grafana/pyroscope-squad#454
@simonswine simonswine force-pushed the 20250612_artificial-delay branch from f11b7e4 to bd040b5 Compare June 16, 2025 08:07
@simonswine simonswine merged commit 4dd52b2 into main Jun 16, 2025
24 checks passed
@simonswine simonswine deleted the 20250612_artificial-delay branch June 16, 2025 09:12
simonswine added a commit that referenced this pull request Jun 16, 2025
* feat: Add artificial ingestion delay

This implements the ability to add artificial delay to the ingestion handlers.

This is useful during the v2 migration process in order to spot how client will behave

Fixes grafana/pyroscope-squad#454

* Remove unnecessary ctx variable

* Use unimplemented embedding
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