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

feat(metrics): support OpenMetrics from applications #7103

Closed
wants to merge 4 commits into from

Conversation

AyushSenapati
Copy link
Contributor

Checklist prior to review

closes: #7093

If mediatype is text/plain, return Prometheus text format without
checking the version param.

Signed-off-by: AyushSenapati <[email protected]>
If application supports openmetrics format, attempt to make
envoy stats compatible with openmetrics format

Signed-off-by: AyushSenapati <[email protected]>
@AyushSenapati AyushSenapati changed the title Openmetrics merge feat(kuma-dp): support OpenMetrics fmt for metrics merge Jun 24, 2023
@michaelbeaumont michaelbeaumont changed the title feat(kuma-dp): support OpenMetrics fmt for metrics merge feat(metrics): support OpenMetrics from applications Jun 26, 2023
Copy link
Contributor

@michaelbeaumont michaelbeaumont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AyushSenapati can you do me a favor and base this PR off the reverted commit? So the first commit would be a revert of the revert. A lot of the original PR still makes sense and it'd make it easier to review what's new since that PR.

@@ -342,6 +376,7 @@ func responseFormat(h http.Header) expfmt.Format {
case textType:
return expfmt.FmtText

// TODO: if version does not match or set, should we return FmtUnknown?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can expect the open metric format to have a version I think.

@bartsmykla
Copy link
Contributor

@AyushSenapati can we close this PR in favor of #7125 then?

@AyushSenapati
Copy link
Contributor Author

AyushSenapati commented Jun 27, 2023

@bartsmykla Yes we can close this PR. I have updated #7125. Both #7103 and #7125 contain same code, but #7125 history is cleaner and will be easier for you to review.

We can also change the title of #7125 to match with #7103

@bartsmykla bartsmykla closed this Jun 27, 2023
@AyushSenapati AyushSenapati deleted the openmetrics-merge branch June 29, 2023 18:53
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.

Handle OpenMetrics prometheus metrics from applications
3 participants