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

Review and enhance metric support for frontend (Go) #70

Closed
6 tasks
cartersocha opened this issue May 30, 2022 · 7 comments · Fixed by #160
Closed
6 tasks

Review and enhance metric support for frontend (Go) #70

cartersocha opened this issue May 30, 2022 · 7 comments · Fixed by #160
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@cartersocha
Copy link
Contributor

The following are requirements for metrics in the project. Not all services will meet all requirements.

  • Service metrics should not be duplicative of trace-derived metrics.
  • Services should emit multiple business metrics where appropriate.
  • Services should emit trace exemplars for their metrics.
  • Services should use push metrics where possible.
  • Services should explicitly read baggage where needed.
  • Collectors should perform metric transforms to normalize resource and service metrics.

Referencing issue: #43

@cartersocha cartersocha added enhancement New feature or request help wanted Extra attention is needed required-for-release labels May 30, 2022
@cartersocha cartersocha changed the title Review and enhance tracing support for frontend (Go) Review and enhance metric support for frontend (Go) May 30, 2022
@fatsheep9146
Copy link
Contributor

hi, I want to help with this, can you assign this to me? @cartersocha

@cartersocha
Copy link
Contributor Author

@fatsheep9146 🚀

@fatsheep9146
Copy link
Contributor

fatsheep9146 commented Jun 22, 2022

It seems that this issue is auto closed by #160? I think it is still needed to be open. @cartersocha

@cartersocha cartersocha reopened this Jun 22, 2022
@cartersocha
Copy link
Contributor Author

I think pr merges close referenced issues by default. It’s a bit annoying. Thanks for calling this out & for starting the metrics work

@julianocosta89
Copy link
Member

I think pr merges close referenced issues by default. It’s a bit annoying. Thanks for calling this out & for starting the metrics work

Whenever the PR has fix #<ISSUE_NUMER> the issue will be automatically closed.
When not implementing everything we could just refer the issue, but without the fix to avoid this behavior.

@fatsheep9146
Copy link
Contributor

After a few days of learning, I found that the metric for frontend service still have many things need to be done,

  1. The existing metrics do not match the Semantic Conventions for Http Server, the gaps includes
  • missed metrics, such as http.server.request.size
  • redundant metrics, such as http.server.request_count
  • missed labels, such as http.method for http.server.duration(for now only http.flavor/scheme/server_name/host exists).
  1. The existing instrumentation library is not good enough, for example

  2. Do not use instrumentation library as much as possible, for example net/http has its own instrumentation library otelhttp

And I think those problems are also common for other services.

How to solve those problems? I have thought of following steps:

1. 'Matching Semantic Conventions' is first priority.

Let the service exposes the metric which matches conventions is very important, because this can let the user immediately experience the power of opentelemetry.

To make this, we should use existing intrumentation library as much as possible. But what if the instrumentation library is also not good enough to follow semantic conventions? Here comes step 2.

2. Use manual instrumentation if instrumentation library is not good enough.

If instrumentation library is not good enough, for example, instrumentation library for instrumentation library for gorilla does not support metric, we should not just wait instrumentation library to become mature, but should directly implements the metrics by manual instrumentation.

#175 is an example.

In this way, we can let our user to experiment the charm of otel, but no need to care about the detail.

But manual instrumentation is not a longterm solution, so here comes step 3

3. Improve the instrumentation library and replace with it until it is good enough

We can open issue or make contributations directly to instrumentation library to enhance the library. For example, I already make a pull request open-telemetry/opentelemetry-go#3018 about adding the missing http.method label to metric of net/http.

When library is good enough, we could consider remove all manual instrumentation codes and use instrumentation library directly, which is also a good practise for trace.

Sorry for my comment is a little bit long, but I ready hope for your ideas :-)
@julianocosta89 @cartersocha @reyang @austinlparker @mic-max

@cartersocha
Copy link
Contributor Author

Closing until front end is overhauled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
No open projects
3 participants