-
Notifications
You must be signed in to change notification settings - Fork 795
feat: Expose Prometheus metrics. Closes #204 #1047
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
Conversation
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
eventsources/eventing.go
Outdated
| continue | ||
| } | ||
| wg.Add(1) | ||
| e.metrics.IncRunningServices(server.GetEventSourceName()) |
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.
move one line down so code change cannot create regression?
docs/metrics.md
Outdated
| ### EventSource | ||
| - `event_service_running_total` - How many configured events in the EventSource |
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 I ask for a section state which metrics is which Golden Metric?
Signed-off-by: Derek Wang <[email protected]>
docs/metrics.md
Outdated
| ### EventSource | ||
| - `event_service_running_total` - How many configured events in the EventSource |
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.
in Argo Workflow the Prometheus HELP contains deep-links to the docs, you could do this same here, allowing you to change the help with a release
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
docs/metrics.md
Outdated
|
|
||
| - Saturation | ||
|
|
||
| - TBD. |
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.
this is probably covered by out of the box Kubernetes metrics such as CPU or memory (RSS)
docs/metrics.md
Outdated
|
|
||
| - Latency | ||
|
|
||
| - TBD. |
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.
"The time it takes to service a request. "
What is a request in events?
- Event Source - the time from getting event to send message to EventBus?
- EventBus - the time from receive a NATS message to dispatching it?
- Sensor - the time from receiving event to getting trigger finishing?
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.
- Added the latency for Sensor triggering actions.
- Event Source latency would be added in future PRs as it requires to make code change to every event source implementation.
- EventBus latency needs further investigation - we could not calculate it from the clients side (when eventsource sends it, and when sensor receives it), but should look into if NATS exposes this kind of metrics, I haven't seen it so far, need more research.
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
alexec
left a comment
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.
please set-up 20m
|
|
||
| > v1.3 and after | ||
| ## User 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.
"system" or "default metrics"?
| #### argo_events_event_processing_duration_milliseconds | ||
| Event processing duration (from getting the event to send it to EventBus) in | ||
| milliseconds - TBD. |
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.
TBD?
* feat: Expose Prometheus metrics. Closes argoproj#204 Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang [email protected]
Closes #204
Checklist: