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

365 akka http connection metrics #404

Merged
merged 7 commits into from
May 18, 2022
Merged

Conversation

lgajowy
Copy link
Contributor

@lgajowy lgajowy commented May 11, 2022

Closes #365


Summary:

  • adds http connections metric with it's config
  • adds grafana dashboard for it

@lgajowy
Copy link
Contributor Author

lgajowy commented May 11, 2022

@worekleszczy Some unit/integration testing is still needed - i need to figure out how to do it best

@lgajowy lgajowy requested a review from worekleszczy May 11, 2022 11:05
@lgajowy
Copy link
Contributor Author

lgajowy commented May 13, 2022

@worekleszczy Regarding adding testing to this pr - it seems to be a bigger research than I initially anticipated:

  • maybe it's possible to use the context similar to what you did in pr 366 path attribute updated #402. if we can use otel context to assert the fact that some connection-related attributes appeared, that could lead us somewhere. Sounds a little bit hacky in this particular case, though.

  • it would be a lot better to be have some test MetricsSDK used by the AgentForTesting. The instrument instances (eg. connections counter) would assert if the value was passed to them in the test. No idea if this is doable.

  • I noticed that there are methods like below in the AgentTestRunner:

testRunner.runWithClientSpan()
testRunner.runWithServerSpan()

this looks like something useful to assert if the span was created correctly. We do not use spans however but maybe there's something similar for metrics alone?

Copy link
Contributor

@worekleszczy worekleszczy left a comment

Choose a reason for hiding this comment

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

LGTM

@worekleszczy worekleszczy merged commit 391e356 into main May 18, 2022
@lgajowy lgajowy deleted the 365-akka-http-connection-metrics branch May 30, 2022 09:01
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.

Reinstate Akka http connections metric
2 participants