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

The metrics exposed by dkron are only of summary type and cannot cover related analysis for specific jobs. #1310

Open
cobolbaby opened this issue Apr 3, 2023 · 5 comments · Fixed by #1389

Comments

@cobolbaby
Copy link
Contributor

cobolbaby commented Apr 3, 2023

Is your feature request related to a problem? Please describe.

The metric dkron_grpc_client_agent_run needs to add some necessary labels, such as job_name...

Describe the solution you'd like
A clear and concise description of what you want to happen.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@cobolbaby cobolbaby changed the title /metrics does not expose job-related metrics The metrics exposed by dkron are only of summary type and cannot cover related analysis for specific jobs. Sep 24, 2023
@cobolbaby
Copy link
Contributor Author

I still prefer to trace related metrics such as dkron_job_execution_time_seconds and dkron_job_execution_done_count in the dkron server, so that when using prometheus for monitoring, I can configure fewer endpoints and only collect server role nodes.

@cobolbaby
Copy link
Contributor Author

cobolbaby commented Nov 13, 2023

It would be best to change the following code:

defer metrics.MeasureSince([]string{"grpc", "execution_done"}, time.Now())

to:

jobLabel := metrics.Label{Name: "job_name", Value: execDoneReq.Execution.JobName}
defer metrics.MeasureSinceWithLabels([]string{"grpc", "execution_done"}, time.Now(), []metrics.Label{jobLabel})

@cobolbaby
Copy link
Contributor Author

@vcastellm Please reopen it. The current implementation in plugin is not elegant. I will try the way mentioned above.

@vcastellm vcastellm reopened this Dec 9, 2023
@vcastellm
Copy link
Member

Can you work on a follow up PR or should I revert?

@cobolbaby
Copy link
Contributor Author

Can you work on a follow up PR or should I revert?

I will follow up. Thanks.

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 a pull request may close this issue.

2 participants