Don't over abbreviate rejections in compact record logger #10156
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
The compact record logger abbreviates long rejection reasons. This makes sense, because these can be excessively long in some cases. However, the compact record logger shouldn't reduce the usefulness of this data. Take for example the following compacted record log: ``` -------- C DPLY CREATE - #1->-1 -1 - R DPLY CREATE - #2->#1 -1 - !INVALID_ARGUMENT (Expected to deploy new resources, but encountered the following errors: 'process.xml': - Element: message_8722b478-fc0f-4c67-82fe-46ee7c067da2 > extensionElements > subscription - ERROR: Expecte..) -------------- Deployed Processes ---------------------- ``` Is it clear what the problem is and what the user did wrong? Well, this happened to me while writing a test and I didn't know what I'd done wrong. This commit allows for a little bit more breathing room to show the rejection reason, while still abbreviating very long rejections. The magic number 500 is hand-picked for no particular reason. ``` -------- C DPLY CREATE - #1->-1 -1 - R DPLY CREATE - #2->#1 -1 - !INVALID_ARGUMENT (Expected to deploy new resources, but encountered the following errors: 'process.xml': - Element: message_252ebafa-ed9a-40a6-be98-ec31e71316a8 > extensionElements > subscription - ERROR: Expected expression but found static value 'key'. An expression must start with '=' (e.g. '=key'). ) -------------- Deployed Processes ---------------------- ``` Now, it's completely clear what my mistake was. I tried to use a static value as correlation key, but it must be an expression. ¯\_(ツ)_/¯ We could consider further abbreviating this rejection message in a smart way, but for now this seems enough.
korthout
force-pushed
the
korthout-rejection-abbreviation
branch
from
4fd2c4e
to
530bdb0
Compare
korthout
changed the title
|
bors merge |
|
Build succeeded: |
|
Successfully created backport PR #10157 for |
This was referenced Aug 24, 2022
|
Successfully created backport PR #10158 for |
10 tasks
zeebe-bors-camunda bot
added a commit
that referenced
this pull request
Aug 24, 2022
10170: Print record timestamp in compact record logger r=korthout a=korthout ## Description <!-- Please explain the changes you made here. --> When troubleshooting issues (or writing tests) for Timer related topics, we're often interested in both the Timer record's Duedate, as well as the actual time that the record was written, i.e. the record's Timestamp. This PR adds the Timestamp to the compact record logger when there are Timer records on the recorded log. While working on this, I also noticed that the compact record logger's Legend could be improved to only include Partition related info when the partition is actually printed as well (i.e. when there are multiple partitions). This is another PR in a series of QoL improvements I've recently made to the compact record logger. - #10156 - #10144 - #9885 ### Before ``` -------- [Partition] ['C'ommand/'E'event/'R'ejection] [valueType] [intent] - #[position]->#[source record position] P[partitionId]K[key] - [summary of value] P9K999 - key; #999 - record position; "ID" element/process id; `@"elementid"/[P9K999]` - element with ID and key Keys are decomposed into partition id and per partition key (e.g. 2251799813685253 -> P1K005). If single partition, the partition is omitted. Long IDs are shortened (e.g. 'startEvent_5d56488e-0570-416c-ba2d-36d2a3acea78' -> 'star..acea78' -------- C DPLY CREATE - #1->-1 -1 - E PROC CREATED - #2->#1 K1 - process.xml -> "process" (version:1) E TIME CREATED - #3->#1 K3 - `@"start_1"[-1]` in <process "K1"[?]> due T15:10:34.329 E DPLY CREATED - #4->#1 K2 - process.xml E DPLY FULLY_DISTRIBUTED - #5->#1 K2 - ``` ### After With Timer records: ``` -------- [Timestamp] ['C'ommand/'E'event/'R'ejection] [valueType] [intent] - #[position]->#[source record position] K[key] - [summary of value] P9K999 - key; #999 - record position; "ID" element/process id; `@"elementid"/[P9K999]` - element with ID and key Keys are decomposed into partition id and per partition key (e.g. 2251799813685253 -> P1K005). If single partition, the partition is omitted. Long IDs are shortened (e.g. 'startEvent_5d56488e-0570-416c-ba2d-36d2a3acea78' -> 'star..acea78' -------- 15:28:01 C DPLY CREATE - #1->-1 -1 - 15:28:02 E PROC CREATED - #2->#1 K1 - process.xml -> "process" (version:1) 15:28:02 E TIME CREATED - #3->#1 K3 - `@"start_1"[-1]` in <process "K1"[?]> due T15:28:03.087 15:28:02 E DPLY CREATED - #4->#1 K2 - process.xml 15:28:02 E DPLY FULLY_DISTRIBUTED - #5->#1 K2 - ``` Without Timer records: ``` -------- ['C'ommand/'E'event/'R'ejection] [valueType] [intent] - #[position]->#[source record position] K[key] - [summary of value] P9K999 - key; #999 - record position; "ID" element/process id; `@"elementid"/[P9K999]` - element with ID and key Keys are decomposed into partition id and per partition key (e.g. 2251799813685253 -> P1K005). If single partition, the partition is omitted. Long IDs are shortened (e.g. 'startEvent_5d56488e-0570-416c-ba2d-36d2a3acea78' -> 'star..acea78' -------- C DPLY CREATE - #1-> -1 -1 - E PROC CREATED - #2->#1 K01 - process.xml -> "process" (version:1) E DPLY CREATED - #3->#1 K02 - process.xml E DPLY FULLY_DISTRIBUTED - #4->#1 K02 - ``` When there are multiple partitions: ``` -------- [Partition] ['C'ommand/'E'event/'R'ejection] [valueType] [intent] - #[position]->#[source record position] P[partitionId]K[key] - [summary of value] P9K999 - key; #999 - record position; "ID" element/process id; `@"elementid"/[P9K999]` - element with ID and key Keys are decomposed into partition id and per partition key (e.g. 2251799813685253 -> P1K005). If single partition, the partition is omitted. Long IDs are shortened (e.g. 'startEvent_5d56488e-0570-416c-ba2d-36d2a3acea78' -> 'star..acea78' -------- 1 C DPLY CREATE - #1->-1 -1 - 1 E PROC CREATED - #2->#1 P1K1 - process.bpmn -> "shouldR..ecovery" (version:1) 1 E DPLY CREATED - #3->#1 P1K2 - process.bpmn 1 E DSTR DISTRIBUTING - #4->#1 P1K2 - on partition 2 1 E DSTR DISTRIBUTING - #5->#1 P1K2 - on partition 3 1 C DSTR COMPLETE - #6->-1 P1K2 - on partition 2 1 C DSTR COMPLETE - #7->-1 P1K2 - on partition 3 1 E DSTR COMPLETED - #8->#6 P1K2 - on partition 2 1 E DSTR COMPLETED - #9->#7 P1K2 - on partition 3 1 E DPLY FULLY_DISTRIBUTED - #10->#7 P1K2 - 2 C DPLY DISTRIBUTE - #1->-1 P1K2 - process.bpmn 2 E DPLY DISTRIBUTED - #2->#1 P1K2 - process.bpmn 3 C DPLY DISTRIBUTE - #1->-1 P1K2 - process.bpmn 3 E DPLY DISTRIBUTED - #2->#1 P1K2 - process.bpmn ``` ## Related issues <!-- Which issues are closed by this PR or are related --> relates to #10169 Co-authored-by: Nico Korthout <nico.korthout@camunda.com>
This was referenced Aug 24, 2022
saig0
added
release/8.0.8
version:1.3.14
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
The compact record logger abbreviates long rejection reasons. This makes sense, because these can be excessively long in some cases. However, the compact record logger shouldn't reduce the usefulness of this data.
Before:
After:
We could consider further abbreviating this rejection message in a smart way, but for now this seems enough.
Related issues
NA
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/1.3) to the PR, in case that fails you need to create backports manually.Testing:
Documentation:
Please refer to our review guidelines.