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 #7125

Merged
merged 12 commits into from
Jul 18, 2023

Conversation

AyushSenapati
Copy link
Contributor

This reverts commit bba743f.

Checklist prior to review

…umahq#7092)"

This reverts commit bba743f.

Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
@AyushSenapati AyushSenapati requested a review from a team as a code owner June 26, 2023 17:41
@AyushSenapati AyushSenapati requested review from Automaat and bartsmykla and removed request for a team June 26, 2023 17:41
If mediatype is text/plain, return Prometheus text format without
checking the version param.

Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
If application supports openmetrics format, attempt to make
envoy stats compatible with openmetrics format

Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
@michaelbeaumont michaelbeaumont changed the title Revert "Revert "fix(kuma-dp): honour app content-type (#6783)" (#7092)" feat(metrics): support OpenMetrics from applications Jun 27, 2023
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
Instead of converting bytes to string and use regex to dedup
new line characters and trim spaces, do the processing
on byte data.

Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
In case of FmtUnknown, as we are using Negotiate instead of
NegotiateWithOpenmetrics, change the expected content type
to FmtText from FmtOpenMetrics

Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
Process Metrics already tests what processNewlineChars function tests.
So remove the unit test for processNewlineChars function.

Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
Copy link
Contributor

@bartsmykla bartsmykla left a comment

Choose a reason for hiding this comment

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

That's my initial review as I want to have a look once again later today

app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
- Use expfmt exposed consts instead of custom defined ones.
- Do not handle err returned by buf.Write() as it is always nil.

Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
app/kuma-dp/pkg/dataplane/metrics/server.go Outdated Show resolved Hide resolved
Signed-off-by: AyushSenapati <a.p.senapati008@gmail.com>
@bartsmykla
Copy link
Contributor

@AyushSenapati I'm gonna test if everything works, and if @michaelbeaumont won't have anything against we could merge it

@bartsmykla
Copy link
Contributor

Ok, tested with kumactl install demo --without-gateway and with kumactl install observability. It looks like everything works
image

@bartsmykla bartsmykla merged commit 7f7f96a into kumahq:master Jul 18, 2023
3 checks passed
@bartsmykla
Copy link
Contributor

Once again @AyushSenapati thank you for your contribution!

@AyushSenapati AyushSenapati deleted the revert-honour-app-contenttype branch September 19, 2023 14:15
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