Skip to content

Conversation

@kylemumma
Copy link
Member

@kylemumma kylemumma commented Nov 10, 2025

I am beginning to rollout GetTrace endpoint pagination, see this PR. Right now the user must opt-in to pagination by providing a limit. Eventually we would like to enforce a global max limit that is always enforced, but before we can do that we need all of our users to be ready to support pagination. I.e. they need to be able to handle receiving the results in multiple pages.

I will tell product to add this support, and then begin sending a limit in their request. Once all user queries contain a limit, I can enable global max.

@kylemumma kylemumma requested review from a team as code owners November 10, 2025 19:44
)
else:
self.metrics.increment(
"eap_trace_request_without_limit", 1, tags={"referrer": in_msg.meta.referrer}
Copy link
Contributor

Choose a reason for hiding this comment

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

should the second argument here be 0?

Copy link
Member Author

Choose a reason for hiding this comment

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

no because they are 2 different metrics

Copy link
Contributor

Choose a reason for hiding this comment

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

no because they are 2 different metrics

got it, I think you should not bother with the "limit not present" metric. because that should always be count(total requests) - count(limit present requests)

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not make this one metric with a different tag for limit/no limit? Is it easier to have 2 metrics?

@phacops
Copy link
Contributor

phacops commented Nov 10, 2025

then begin sending limit=-1

I don't know if I'd ask them to make a change then tell them to undo that change. I'd ask them to change and agree on a date by when you'd enable this.

With the metrics you're adding, you'd be able to track down which referrer is not querying with a limit and tell them to update and at some point, you'll just enable that for good.

@kylemumma kylemumma merged commit 7de88bf into master Nov 11, 2025
46 of 50 checks passed
@kylemumma kylemumma deleted the krm/limitmetric branch November 11, 2025 21:41
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.

4 participants