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 request start time metrics to request header field #647

Merged
merged 6 commits into from
May 10, 2024

Conversation

bhakiyakalimuthu
Copy link
Contributor

@bhakiyakalimuthu bhakiyakalimuthu commented Apr 24, 2024

📝 Summary

This PR introduces the ability to add the request start time in Unix milliseconds and the start time into the slot to the request headers. This enhancement facilitates the measurement of latency between the validator and the relay.

⛱ Motivation and Context

By adding these time metrics into the headers, we can better analyze the causes of request delays, determining whether they are due to network latency or performance issues with the validator node.

✅ I have run these commands

  • make lint
  • make test-race
  • go mod tidy

server/service.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@jtraglia jtraglia left a comment

Choose a reason for hiding this comment

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

This seems reasonable 👍 good idea.

Can we add back the deleted blank lines? This is just a nit.

@bhakiyakalimuthu
Copy link
Contributor Author

bhakiyakalimuthu commented Apr 26, 2024

This seems reasonable 👍 good idea.

Can we add back the deleted blank lines? This is just a nit.

Those lines were deleted for a reason. Linter was throwing error as below if the lines were not removed.
server/service.go:295:1: Function name: handleGetHeader, Cyclomatic Complexity: 24, Halstead Volume: 5978.93, Maintainability Index: 19 (maintidx) func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) { ^ make: *** [lint] Error 1

@avalonche
Copy link
Collaborator

interesting that removing blank lines makes the linter think there is less complexity. the function probably needs refactoring

@bhakiyakalimuthu
Copy link
Contributor Author

bhakiyakalimuthu commented May 6, 2024

interesting that removing blank lines makes the linter think there is less complexity. the function probably needs refactoring

Hi @avalonche Is there anything else we need to address, or are we ready to merge this PR?

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