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

add stats handler #218

Merged
merged 2 commits into from
Jul 3, 2024
Merged

Conversation

anuvedverma
Copy link
Contributor

No description provided.

@anuvedverma
Copy link
Contributor Author

/ptal @riteshvaryani @theevocater

Very rough attempt at adding StatisticsHandler logs-- lmk if you know a better way to do this!

@anuvedverma anuvedverma merged commit 6343539 into sev-16337-add-header-logging Jul 3, 2024
3 checks passed
@anuvedverma anuvedverma deleted the add-stats-handler branch July 3, 2024 23:15
StatisticsHandler stats = this.server.getChildHandlerByClass(StatisticsHandler.class);
this.scheduler.scheduleAtFixedRate(() -> {
log.debug("(jetty) Num requests: " + stats.getRequests());
log.debug("(jetty) Num requests: " + stats.getRequestsActive());

Choose a reason for hiding this comment

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

Num active requests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah thanks-- fixed it on the sev-16337-add-header-logging branch

@theevocater
Copy link

wait i'm confused -- is the goal here to actually collect these metrics or just log them to stdout? wouldn't it be better to just output this via the statsd collector we already have?

@anuvedverma
Copy link
Contributor Author

@theevocater doing this via statsd collector would probably be much better, but I'm honestly not sure how to do it... I see that we use statsd collector in prestoproxy, but don't see it in presto-gateway. Or would the idea be to configure the StatisticsHandler request logging somewhere in prestoproxy instead?

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.

3 participants