-
Notifications
You must be signed in to change notification settings - Fork 642
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
[ISSUE #340]Integrate With OpenTelemetry for metrics in EventMesh #467
Conversation
# Conflicts: # eventmesh-runtime/build.gradle
Codecov Report
@@ Coverage Diff @@
## develop #467 +/- ##
============================================
- Coverage 9.84% 9.80% -0.04%
Complexity 283 283
============================================
Files 228 230 +2
Lines 10829 10870 +41
Branches 923 923
============================================
Hits 1066 1066
- Misses 9666 9707 +41
Partials 97 97
Continue to review full report at Codecov.
|
|
||
//maxHTTPCost | ||
meter | ||
.longValueObserverBuilder("max.HTTPCost") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's strange to spell metric with hump.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how should I spell?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eventmesh.http.request.elapsed.max
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get it
@qqeasonchen I found there is many metric in eventmesh now. Maybe it is a good opportunity to make them in otel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job, anyway. I think is it a good start for your open source journy。
btw, your document can be formatted with some tools. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible for us to use plugin implementation of exporter? I think maybe not everyone will use prometheus.
List open_telemetry = [ | ||
"io.opentelemetry:opentelemetry-api:1.3.0", | ||
"io.opentelemetry:opentelemetry-sdk:1.3.0", | ||
"io.opentelemetry:opentelemetry-sdk-metrics:1.3.0-alpha", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use other stable version? Using alpha does not seem to be a good choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is ok to use that. Otel's sdk metrics is in alpha although, it is widely used in https://github.com/open-telemetry/opentelemetry-java-instrumentation. This agent has been validated in enough prod environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
|
||
private HTTPServer server;//Prometheus server | ||
|
||
int prometheusPort = 19090;//the endpoint to export metrics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to add this to configuration properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've thought about it, but I'm not very good at it. What language is involved? script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most configuration is at eventmesh-runtime/conf/eventmesh.properties
, in the future, I think it may be split into multiple files, but now you can just add the prometheusPort in this file, and load this config, you can see code in EventMeshHTTPConfiguration
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will have a try.
what kind of tools? I have no experience, but I will try my best to do that. |
@Roc-00 https://doocs.gitee.io/md/#/ |
yep. |
@Roc-00 Maybe the metrics about tcp in 'EventMeshTcpMonitor.java' can also be processed in the same way. |
Is this similar to typora |
I will find out the difference between HTTP and TCP,then complete TCP and HTTP metrics |
@Roc-00 please re-submit this pr to branch opentelemetry, and add a design doc first like https://github.com/apache/incubator-eventmesh/blob/develop/docs/en/features/eventmesh-stream-design.md. |
OK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Roc-00重请提交这个pr 到分支开放遥测,并首先添加一个设计文档,如https://github.com/apache/incubator-eventmesh/blob/develop/docs/en/features/eventmesh-stream-design .MD。
OK.
please avoid chinese here.
@@ -0,0 +1,41 @@ | |||
# Open Telemetry exporter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readme file should not exist in src
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I delete it,or put it in the design doc
@@ -0,0 +1,46 @@ | |||
# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yml file should not exist in src
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think is ok.
Should I close this PR so that I can re-submit this pr to branch opentelemetry? |
you can edit this pr in this page, change the branch you want to merge |
* [ISSUE #340]Integrate With OpenTelemetry for metrics in EventMesh (#467) * The Chinese in the notes is translated into English. * Translation improvement * Translation improvement * fix Chinese annotation on runtime module * Export metrics data with open telemetry and use Prometheus for visual observation * Export metrics data with open telemetry and use Prometheus for visual observation * Export metrics data with open telemetry and use Prometheus for visual observation * Export metrics data with open telemetry and use Prometheus for visual observation * Improper modification * Improper modification * improve * improve * improve * tcp metrics export * improve * improve * merge from otel branch * check lisence Co-authored-by: ZePeng Chen <84842773+Roc-00@users.noreply.github.com>
* [ISSUE apache#340]Integrate With OpenTelemetry for metrics in EventMesh (apache#467) * The Chinese in the notes is translated into English. * Translation improvement * Translation improvement * fix Chinese annotation on runtime module * Export metrics data with open telemetry and use Prometheus for visual observation * Export metrics data with open telemetry and use Prometheus for visual observation * Export metrics data with open telemetry and use Prometheus for visual observation * Export metrics data with open telemetry and use Prometheus for visual observation * Improper modification * Improper modification * improve * improve * improve * tcp metrics export * improve * improve * merge from otel branch * check lisence Co-authored-by: ZePeng Chen <84842773+Roc-00@users.noreply.github.com>
* [ISSUE #340]Integrate With OpenTelemetry for metrics in EventMesh (#467) * The Chinese in the notes is translated into English. * Translation improvement * Translation improvement * fix Chinese annotation on runtime module * Export metrics data with open telemetry and use Prometheus for visual observation * Export metrics data with open telemetry and use Prometheus for visual observation * Export metrics data with open telemetry and use Prometheus for visual observation * Export metrics data with open telemetry and use Prometheus for visual observation * Improper modification * Improper modification * improve * improve * improve * tcp metrics export * improve * improve * merge from otel branch * check lisence Co-authored-by: ZePeng Chen <84842773+Roc-00@users.noreply.github.com>
Fixes ISSUE#<340>.
Motivation
Export metrics data with open telemetry and use Prometheus for visual observation
Modifications
There is a readme document in the code I submitted. I use idea to run locally. I download Prometheus in the window version of the official website. The operation method of docker written in readme is given by reference to open telemetry. I haven't actually operated it. I was successful in running locally. I'm not sure whether the code I wrote meets the requirements, so I didn't finish all the metrics, but wrote three. If there is anything wrong, please put it forward and I will modify and improve it. When the code is perfect and correct, I will continue to write the rest of the code.
Documentation