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

replace the metrics in HttpServerListener with Micrometer #455

Closed
wants to merge 5 commits into from

Conversation

7ue5wu
Copy link

@7ue5wu 7ue5wu commented Jun 7, 2023

Context

Try to replace the metrics part in HttpServerListener with Micrometer metrics and wanna get some early feedback and advice.

Explanation:
In the previous implementation, we have the concepts GroupId(name, tags) and MetricId(groupname, metricname, tags).
However, in the Micrometer, it cannot name the Metrics with GroupId, so I try to add the group name as a prefix in metric name and connect them with the underscore.
If we need the group name or the metric(event) name when publishing, we can extract it from the whole metric name.
And we can still use them to form a GroupId(groupname + tags). and MetricId(metricName + tag) if needed.

Checklist

  • ./gradlew build compiles code correctly
  • Added new tests where applicable
  • ./gradlew test passes all tests
  • Extended README or added javadocs where applicable

@7ue5wu 7ue5wu had a problem deploying to Integrate Pull Request June 7, 2023 14:32 — with GitHub Actions Failure
@7ue5wu 7ue5wu had a problem deploying to Integrate Pull Request June 9, 2023 07:07 — with GitHub Actions Failure
@Andyz26
Copy link
Collaborator

Andyz26 commented Jun 12, 2023

@hmit I suppose these change cannot be merged in master directly as it will break the dashboard etc. once picked up by nfmantis? Shall we keep all metrics changes to a feature branch for now?

@hmitnflx
Copy link
Collaborator

@Andyz26 this is an initial PR to get early feedback on implementation details for micrometer migration. The other piece to make the metrics inter-operational is still being worked out.

@7ue5wu 7ue5wu had a problem deploying to Integrate Pull Request June 15, 2023 03:53 — with GitHub Actions Failure
@7ue5wu 7ue5wu closed this Sep 5, 2023
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.

None yet

3 participants