feat: Introduce operational metrics in OpenTelemetry Collector #4860

SpiritZhou · 2023-08-04T08:28:58Z

Refactor the current Prometheus.go file by adding a middleware metriccollector.go to handle initialization and metrics collection for both Prometheus and OpenTelemetry.

About e2e test:

Add opentelemetry install in setup_test.go. This opentelemetry has promethues exporter.
Add env and flag when KEDA is running in e2e test env.
Fecth metric from opentelemetry through promethues port to test

Checklist

I have verified that my change is according to the deprecations & breaking changes policy
Tests have been added
Changelog has been updated and is aligned with our changelog requirements
A PR is opened to update the documentation on (repo) (if applicable)
Commits are signed with Developer Certificate of Origin (DCO - learn more)

Relates to #3078
Relates to kedacore/keda-docs#1215

Signed-off-by: SpiritZhou <[email protected]>

github-actions · 2023-08-04T08:29:08Z

Thank you for your contribution! 🙏 We will review your PR as soon as possible.

🏖️ Over the summer, the response time will be longer than usual due to maintainers taking time off so please bear with us.

While you are waiting, make sure to:

Add an entry in our changelog in alphabetical order and link related issue
Update the documentation, if needed
Add unit & e2e tests for your changes
GitHub checks are passing
Is the DCO check failing? Here is how you can fix DCO issues

Learn more about:

Our contribution guide

Signed-off-by: SpiritZhou <[email protected]>

pkg/metricscollector/opentelemetry.go

Signed-off-by: SpiritZhou <[email protected]>

JorTurFer

This is an awesome improvement!! 💪
I have left some comments inline, but apart from them:

As testing otel is a must, I'd move the Kustomization to config/e2e because it's the Kustomization file that we use for deploying e2e tests (for releases we use config/default). It makes easier to maintain the files and also ensures that always is deployed

We also have to document this new option in keda-docs 🙏

tests/helper/helper.go

tests/opentelemetry_setup/otlp.yml

Makefile

CHANGELOG.md

tomkerkhove · 2023-08-24T07:22:32Z

We also need to expose this in our Helm chart but that's a separate PR as well

Co-authored-by: Tom Kerkhove <[email protected]> Signed-off-by: SpiritZhou <[email protected]>

Signed-off-by: SpiritZhou <[email protected]>

JorTurFer · 2023-08-25T06:47:05Z

/run-e2e sequential
Update: You can check the progress here

Signed-off-by: SpiritZhou <[email protected]>

pkg/metricscollector/opentelemetry.go

Signed-off-by: SpiritZhou <[email protected]>

JorTurFer · 2023-09-27T08:02:50Z

+1

Signed-off-by: SpiritZhou <[email protected]>

SpiritZhou · 2023-09-27T10:21:08Z

@zroubalik @JorTurFer Just updated the code. Hope it can meet the ddl.

zroubalik · 2023-09-27T10:30:41Z

@zroubalik @JorTurFer Just updated the code. Hope it can meet the ddl.

@SpiritZhou thanks! I am on it now, do you think you can update Helm charts as well?

zroubalik · 2023-09-27T10:31:09Z

/run-e2e sequential
Update: You can check the progress here

zroubalik · 2023-09-27T11:16:32Z

/run-e2e sequential
Update: You can check the progress here

zroubalik

Unfortunately, prometheus tests is failing:

prometheus_metrics_test.go:336: --- testing scaled object errors ---
    helper.go:567: Deleting template: scaledObjectTemplate
    helper.go:5***9: Applying template: wrongScaledObjectTemplate
    prometheus_metrics_test.go:35***: 
        	Error Trace:	/__w/keda/keda/tests/sequential/prometheus_metrics/prometheus_metrics_test.go:35***
        	            				/__w/keda/keda/tests/sequential/prometheus_metrics/prometheus_metrics_test.go:***67
        	Error:      	Should not be: 0
        	Test:       	TestPrometheusMetrics

zroubalik

@SpiritZhou one more thing, could you please make it an experimental feature? Marking it so in the docs and changelog?

There's nothing wrong with the feature itself. Just for us to be able to change the metrics or nomenclature if needed without the need to keep backwards compatibility.

Signed-off-by: SpiritZhou <[email protected]>

SpiritZhou · 2023-09-27T14:16:11Z

@zroubalik I fixed the test and update the "experimental feature". Could you help to review them again?

tomkerkhove · 2023-09-27T14:17:18Z

/run-e2e sequential
Update: You can check the progress here

zroubalik

LGTM, thanks for the quick fix!

tomkerkhove · 2023-09-27T14:41:17Z

Talked to @SpiritZhou and he'll start Helm chart PR, if he can't finish it today I'll take over tomorrow. That should allow us to merge this one in time

SpiritZhou · 2023-09-27T14:54:55Z

It is strange. The prometheus_metrics_test can be passed in my local minikube env...

Signed-off-by: SpiritZhou <[email protected]>

zroubalik · 2023-09-27T15:12:48Z

/run-e2e sequential
Update: You can check the progress here

Signed-off-by: SpiritZhou <[email protected]>

SpiritZhou · 2023-09-27T16:25:33Z

@zroubalik @JorTurFer @tomkerkhove The failed e2e test may not be caused by OpenTelemetry PR. I found it happened in this PR before (https://github.com/kedacore/keda/actions/runs/6308971553). And I also found that the correct metric value can be retrieved after around 15s. I don't have any idea why it becomes slow.

zroubalik · 2023-09-27T19:57:52Z

@zroubalik @JorTurFer @tomkerkhove The failed e2e test may not be caused by OpenTelemetry PR. I found it happened in this PR before (https://github.com/kedacore/keda/actions/runs/6308971553). And I also found that the correct metric value can be retrieved after around 15s. I don't have any idea why it becomes slow.

Right, it is failing on main as well. So it is not relevant to this PR. You can ignore it in this PR, great work, thanks!

Signed-off-by: SpiritZhou <[email protected]>

tomkerkhove · 2023-09-28T06:43:18Z

I will pick up Helm chart later today

…ore#4860) Co-authored-by: Tom Kerkhove <[email protected]> Signed-off-by: anton.lysina <[email protected]>

Update

b23439c

Signed-off-by: SpiritZhou <[email protected]>

SpiritZhou added 2 commits August 14, 2023 17:00

Update

f95a18e

Signed-off-by: SpiritZhou <[email protected]>

Update

28d85ab

Signed-off-by: SpiritZhou <[email protected]>

github-advanced-security bot found potential problems Aug 16, 2023

View reviewed changes

pkg/metricscollector/opentelemetry.go Fixed Show fixed Hide fixed

SpiritZhou added 5 commits August 16, 2023 17:27

Add testing

a1fd2e0

Signed-off-by: SpiritZhou <[email protected]>

Update

f66aecb

Signed-off-by: SpiritZhou <[email protected]>

Update

02e4550

Signed-off-by: SpiritZhou <[email protected]>

Add vendor files

62c7bb0

Signed-off-by: SpiritZhou <[email protected]>

Update change log

2f65367

Signed-off-by: SpiritZhou <[email protected]>

SpiritZhou marked this pull request as ready for review August 23, 2023 09:54

SpiritZhou requested a review from a team as a code owner August 23, 2023 09:54

SpiritZhou added 4 commits August 23, 2023 18:45

Merge branch 'main' into spiritzhou/opentelemetry

b379854

Signed-off-by: SpiritZhou <[email protected]>

Update

a7bb962

Signed-off-by: SpiritZhou <[email protected]>

Update

d10c395

Signed-off-by: SpiritZhou <[email protected]>

Merge branch 'main' into spiritzhou/opentelemetry

028b334

Signed-off-by: SpiritZhou <[email protected]>

JorTurFer reviewed Aug 24, 2023

View reviewed changes

tests/helper/helper.go Outdated Show resolved Hide resolved

tests/opentelemetry_setup/otlp.yml Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

tomkerkhove reviewed Aug 24, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

tomkerkhove changed the title ~~Add opentelemetry metric collection.~~ feat: Introduce operational metrics in OpenTelemetry Collector Aug 24, 2023

SpiritZhou and others added 2 commits August 24, 2023 17:17

Update CHANGELOG.md

93b5747

Co-authored-by: Tom Kerkhove <[email protected]> Signed-off-by: SpiritZhou <[email protected]>

Update

1ef1b5e

Signed-off-by: SpiritZhou <[email protected]>

tomkerkhove assigned JorTurFer Aug 25, 2023

Remove unnecessary file

5603741

Signed-off-by: SpiritZhou <[email protected]>

SpiritZhou mentioned this pull request Aug 30, 2023

docs: OpenTelemetry Collector metrics kedacore/keda-docs#1215

Merged

1 task

pragmaticivan reviewed Sep 12, 2023

View reviewed changes

pkg/metricscollector/opentelemetry.go Outdated Show resolved Hide resolved

SpiritZhou added 2 commits September 22, 2023 10:30

Merge branch 'main' into spiritzhou/opentelemetry

ee2a1b7

Signed-off-by: SpiritZhou <[email protected]>

Update

ff26408

Signed-off-by: SpiritZhou <[email protected]>

SpiritZhou requested a review from tomkerkhove September 22, 2023 02:52

Merge branch 'main' into spiritzhou/opentelemetry

e9163a2

Signed-off-by: SpiritZhou <[email protected]>

SpiritZhou requested review from JorTurFer and zroubalik September 27, 2023 10:03

Fix

2c3c435

Signed-off-by: SpiritZhou <[email protected]>

zroubalik reviewed Sep 27, 2023

View reviewed changes

zroubalik mentioned this pull request Sep 27, 2023

Release: v2.12 #4994

Closed

12 tasks

SpiritZhou added 2 commits September 27, 2023 21:57

Update

47ca2e1

Signed-off-by: SpiritZhou <[email protected]>

Update

3993cd8

Signed-off-by: SpiritZhou <[email protected]>

zroubalik approved these changes Sep 27, 2023

View reviewed changes

SpiritZhou added 2 commits September 27, 2023 23:09

Add temp log for debuging

29314d6

Signed-off-by: SpiritZhou <[email protected]>

Add temp log for e2e testing

106b942

Signed-off-by: SpiritZhou <[email protected]>

Update

a94ee73

Signed-off-by: SpiritZhou <[email protected]>

Remove testing code

e85e5a2

Signed-off-by: SpiritZhou <[email protected]>

tomkerkhove added the skip-e2e label Sep 28, 2023

tomkerkhove merged commit 95a235b into kedacore:main Sep 28, 2023
20 checks passed

toniiiik pushed a commit to toniiiik/keda that referenced this pull request Jan 15, 2024

feat: Introduce operational metrics in OpenTelemetry Collector (kedac…

340f4f6

…ore#4860) Co-authored-by: Tom Kerkhove <[email protected]> Signed-off-by: anton.lysina <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Introduce operational metrics in OpenTelemetry Collector #4860

feat: Introduce operational metrics in OpenTelemetry Collector #4860

SpiritZhou commented Aug 4, 2023 •

edited by tomkerkhove

Loading

github-actions bot commented Aug 4, 2023

JorTurFer left a comment

tomkerkhove commented Aug 24, 2023

JorTurFer commented Aug 25, 2023 •

edited by github-actions bot

Loading

JorTurFer commented Sep 27, 2023

SpiritZhou commented Sep 27, 2023

zroubalik commented Sep 27, 2023

zroubalik commented Sep 27, 2023 •

edited by github-actions bot

Loading

zroubalik commented Sep 27, 2023 •

edited by github-actions bot

Loading

zroubalik left a comment

zroubalik left a comment

SpiritZhou commented Sep 27, 2023

tomkerkhove commented Sep 27, 2023 •

edited by github-actions bot

Loading

zroubalik left a comment

tomkerkhove commented Sep 27, 2023

SpiritZhou commented Sep 27, 2023

zroubalik commented Sep 27, 2023 •

edited by github-actions bot

Loading

SpiritZhou commented Sep 27, 2023

zroubalik commented Sep 27, 2023

tomkerkhove commented Sep 28, 2023

feat: Introduce operational metrics in OpenTelemetry Collector #4860

feat: Introduce operational metrics in OpenTelemetry Collector #4860

Conversation

SpiritZhou commented Aug 4, 2023 • edited by tomkerkhove Loading

Checklist

github-actions bot commented Aug 4, 2023

JorTurFer left a comment

Choose a reason for hiding this comment

tomkerkhove commented Aug 24, 2023

JorTurFer commented Aug 25, 2023 • edited by github-actions bot Loading

JorTurFer commented Sep 27, 2023

SpiritZhou commented Sep 27, 2023

zroubalik commented Sep 27, 2023

zroubalik commented Sep 27, 2023 • edited by github-actions bot Loading

zroubalik commented Sep 27, 2023 • edited by github-actions bot Loading

zroubalik left a comment

Choose a reason for hiding this comment

zroubalik left a comment

Choose a reason for hiding this comment

SpiritZhou commented Sep 27, 2023

tomkerkhove commented Sep 27, 2023 • edited by github-actions bot Loading

zroubalik left a comment

Choose a reason for hiding this comment

tomkerkhove commented Sep 27, 2023

SpiritZhou commented Sep 27, 2023

zroubalik commented Sep 27, 2023 • edited by github-actions bot Loading

SpiritZhou commented Sep 27, 2023

zroubalik commented Sep 27, 2023

tomkerkhove commented Sep 28, 2023

SpiritZhou commented Aug 4, 2023 •

edited by tomkerkhove

Loading

JorTurFer commented Aug 25, 2023 •

edited by github-actions bot

Loading

zroubalik commented Sep 27, 2023 •

edited by github-actions bot

Loading

zroubalik commented Sep 27, 2023 •

edited by github-actions bot

Loading

tomkerkhove commented Sep 27, 2023 •

edited by github-actions bot

Loading

zroubalik commented Sep 27, 2023 •

edited by github-actions bot

Loading