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 handler name http runtime #558

Merged
merged 2 commits into from
May 15, 2024
Merged

Add handler name http runtime #558

merged 2 commits into from
May 15, 2024

Conversation

filafb
Copy link
Contributor

@filafb filafb commented Apr 12, 2024

What is the purpose of this pull request?

This PR adds labels to the request handlers for the routes added by the node sdk to all applications -/healthcheck, /_metrics and /:account/:workspace/_whoami. Currently, the handler undefined is TOP 5 handler in the metrics for runtime_http_requests_total - ref:

Screenshot 2024-04-23 at 5 34 59 PM

one thing that doens't make sense to me is the scale in this dashboard - I might have queried it wrong, but the intention here is to show the relevance of the undefined events

What problem is this solving?

The handler name is added to the metrics based on ctx.requestHandlerName - ref. The requestHandlerName is added by the nameSpanOperation Middleware, and it's applied to the appHttpHandlers, appEventHandlers and appGraphQLHandlers not to the runtimeHttpHandlers - here.

  • For the appHttpHandlers, it's is applied for both private and public routes, it's added here and here;
  • For the appEventHandlers, it's applied here;
  • For the appGraphQLHandlers, it's applied here.

By adding the requestHandlerName to the context in the routes created by runtimeHttpHandlers we should be able to better filter the http metrics for applications.

@filafb filafb self-assigned this Apr 12, 2024
@filafb filafb marked this pull request as ready for review April 23, 2024 21:39
Copy link

@silvadenisaraujo silvadenisaraujo left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

@filafb filafb merged commit 94f3dee into master May 15, 2024
2 checks passed
@filafb filafb deleted the add-handler-name-http-runtime branch May 15, 2024 17:20
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