-
Notifications
You must be signed in to change notification settings - Fork 556
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
[Backport stable/1.3] Configure record filter for metrics exporter #9425
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While trying to write a test that verifies that a record filter is set on the metrics exporter, I kinda stumbled into writing a test that verifies one of the default behaviors of the metrics exporter: it should be able to observe job lifetime. I was unable to find other tests for this code, only for the metrics exporter configuration. So I felt it made sense to add this test case. (cherry picked from commit c8f1eca)
This file always pops up when I run the broker tests locally. It shouldn't be part of the repository because it contains data about the jqwik local run. (cherry picked from commit e4c4341)
The metrics exporter should define a filter so that the exporter director doesn't unnecessarily read records for the metrics exporter. This test does not yet add any specific recordtype recordvalue combinations that are asserted and will fail anyways, because the metrics exporter does not yet define a filter. However, I wanted to seperate the test setup from the actual filter choices. (cherry picked from commit 8605258)
The metrics exporter shouldn't accept all records, because then the exporter director unnecessarily reads record values that it this exporter won't export anyways. In the past, this has led to the exporter getting stuck when it ran into reading problems like #6442. The metrics exporter is only interested in 3 events (job, job_batch and process instance). (cherry picked from commit db64a95)
Adds test implementation for the exporter API, allowing easier testing of exporters. Configuration/context is kept to a minimum: users can set the configuration that will be instantiated beforehand, but no instantiation is done for them from the provided args. This is a limitation at the moment, to avoid coupling the broker's implementation and this one here. It's sufficient for our own use cases for now. The controller can schedule tasks and process them deterministically. This is done manually, by having users "tick" the controller manually - it will then run all scheduled tasks which have "expired" in order, synchronously. Tasks are kept in memory, even after being executed, so we can still assert properties of previous scheduled, canceled, or executed tasks. (cherry picked from commit 041c522)
Cherrypicking the previous commit introcuced some conficts. To highlight and clarify the resolution choices, I've extracted the resolution into a separate commit. Specifically, the newly created zeebe-exporter-test needs to be specified in the bom and its version should be aligned with the pom versions of stable/1.3.
Version 1.3 does not yet support > 11 language features, so we cannot use static declarations in inner classes and cannot use records. In addition, the ImmutableRecords are not available in 1.3, so I've chosen to completely remove the other test case.
npepinpe
approved these changes
May 25, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
bors merge
zeebe-bors-camunda bot
added a commit
that referenced
this pull request
May 25, 2022
9425: [Backport stable/1.3] Configure record filter for metrics exporter r=npepinpe a=korthout # Description Backport of #9371 to `stable/1.3`. relates to #9240 #6442 Co-authored-by: Nico Korthout <nico.korthout@camunda.com> Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
Build failed: |
bors retry |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Backport of #9371 to
stable/1.3
.relates to #9240 #6442