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

Make collection of Prometheus data be controlled by an annotation. #315

Merged
merged 4 commits into from
Aug 5, 2022

Conversation

wanlin31
Copy link
Collaborator

@wanlin31 wanlin31 commented Jun 29, 2022

Currently we automatically collect Prometheus data for PSM
related tests, now some regular tests (same scenario as PSM
test) are also part of the PSM tests, we may want to collect
Prometheus data for those regular tests too. The current method
for checking if a test isPSM related is to check the existence of
xds-server and sidecar containers.

The default behavior is to not collect any Prometheus data.
This PR adds an annotation enablePrometheus to
elect to collect Prometheus data, once this annotation is
set to true in a LoadTest configuration, the
Prometheus data would be collected for that particular test.

Currently we automatically collect Prometheus data for PSM
related tests, now some regular tests (same scenario as PSM
test) are also part of the PSM tests, we may want to collect
Prometheus data for those regular tests too. The current method
for checking if a rest is a PSM test to check the existence of
xds-server and sidecar containers.
It is hard to apply the same mechanism for PSM related regular
tests,  the simplest way is to add an annotation like
`enablePrometheus:true` to control if we want to use Prometheus
for a given test.
@wanlin31 wanlin31 force-pushed the update-prometheus branch from ae358bc to b5036e9 Compare June 29, 2022 20:53
@wanlin31 wanlin31 self-assigned this Jun 29, 2022
@wanlin31 wanlin31 requested a review from paulosjca June 29, 2022 20:55
@wanlin31 wanlin31 force-pushed the update-prometheus branch 2 times, most recently from 3d2a363 to 8da69cf Compare June 30, 2022 21:49
@wanlin31 wanlin31 force-pushed the update-prometheus branch from 8da69cf to 408e82a Compare June 30, 2022 21:52
@wanlin31 wanlin31 changed the title Make collecting Prometheus data optional through annotation Make collecting Prometheus data default and can be override through annotation Jun 30, 2022
@wanlin31 wanlin31 marked this pull request as ready for review July 1, 2022 00:21
@wanlin31 wanlin31 added the release notes: yes Indicates that PR needs to be in release notes label Jul 1, 2022
Copy link
Collaborator

@paulosjca paulosjca left a comment

Choose a reason for hiding this comment

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

I would also modify the PSM examples to add the enablePrometheus annotation, and modify the kokoro jobs to include it.

I also notice that there is no mention of Prometheus in the README files, we should update the documentation as well (perhaps as a separate PR).

config/constants.go Outdated Show resolved Hide resolved
containers/runtime/driver/run.sh Outdated Show resolved Hide resolved
status/missing_pods_test.go Outdated Show resolved Hide resolved
@wanlin31 wanlin31 force-pushed the update-prometheus branch from 81a181f to 14bd481 Compare August 1, 2022 21:43
@wanlin31
Copy link
Collaborator Author

wanlin31 commented Aug 1, 2022

The loadtest_examples.sh are updated in grpc/grpc#30452

All examples from this PR are generated by loadtest_examples.sh.

@wanlin31 wanlin31 requested a review from paulosjca August 1, 2022 22:10
Copy link
Collaborator

@paulosjca paulosjca left a comment

Choose a reason for hiding this comment

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

This is fine, but you need to update the description to match.

status/missing_pods_test.go Outdated Show resolved Hide resolved
@wanlin31 wanlin31 requested a review from paulosjca August 1, 2022 23:34
@paulosjca paulosjca changed the title Make collecting Prometheus data default and can be override through annotation Make collection of Prometheus data be controlled by an annotation. Aug 5, 2022
@wanlin31 wanlin31 merged commit ef31925 into grpc:master Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: yes Indicates that PR needs to be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants