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 metrics to satisfy http server semantic conventions #232

Closed

Conversation

fatsheep9146
Copy link
Contributor

Signed-off-by: Ziqi Zhao [email protected]

related #70

Changes

Add more metrics to satisfy http server metrics semantic coventions

  • http.server.request.size
  • http.server.response.size
  • http.server.active_requests

http.server.response.size dashboard demo

image

http.server.active_requests dashboard demo

image

@fatsheep9146 fatsheep9146 requested a review from a team July 26, 2022 11:30
Signed-off-by: Ziqi Zhao <[email protected]>
@cijothomas
Copy link
Member

Qn - how does the Go front end export this? Is it done via OTLP exporter? Or via Prometheus directly?

@cijothomas
Copy link
Member

Qn - how does the Go front end export this? Is it done via OTLP exporter? Or via Prometheus directly?

never mind, I think i got the answer from https://github.com/open-telemetry/opentelemetry-demo/blob/main/src/frontend/main.go#L108

@fatsheep9146
Copy link
Contributor Author

Qn - how does the Go front end export this? Is it done via OTLP exporter? Or via Prometheus directly?

never mind, I think i got the answer from https://github.com/open-telemetry/opentelemetry-demo/blob/main/src/frontend/main.go#L108

Yes, via OTLP exporter

@cartersocha
Copy link
Contributor

@fatsheep9146, could you update the changelog and then I'll merge

@cartersocha
Copy link
Contributor

Trying to get better about that upfront 😄. Do any docs need to be updated? I think we have a centralized metric doc but it's a bit immature

@puckpuck
Copy link
Contributor

Would this PR be in conflict with #236 which is replacing frontend with a Next.js based service?

@fatsheep9146
Copy link
Contributor Author

Would this PR be in conflict with #236 which is replacing frontend with a Next.js based service?

Yes, I think @puckpuck is right, since the whole frontend service is replaces with Next.js implementation. Should I close this pr ? @cartersocha

@cartersocha
Copy link
Contributor

Yes let’s wait for the new front end

@fatsheep9146
Copy link
Contributor Author

@cartersocha I think #70 , #49 should also be modified or close to avoid confusion

@cartersocha
Copy link
Contributor

Roger that. Thanks for calling them out. Closed for now

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.

5 participants