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

telemetry: don't resample transactions on retry #119284

Closed
xinhaoz opened this issue Feb 16, 2024 · 0 comments · Fixed by #119481
Closed

telemetry: don't resample transactions on retry #119284

xinhaoz opened this issue Feb 16, 2024 · 0 comments · Fixed by #119481
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@xinhaoz
Copy link
Member

xinhaoz commented Feb 16, 2024

Currently for transaction logging to telemetry we resample transaction executions on automatic retries. We shouldn't do that since they are part of the same execution. The sampling decision should just occur once at the start of the transaction.

Jira issue: CRDB-36126

@xinhaoz xinhaoz added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Feb 16, 2024
@xinhaoz xinhaoz self-assigned this Feb 16, 2024
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Feb 21, 2024
We will only attempt to sample the transaction once at the
start of its execution.

This also moves the sampling decision for transactions to be closer to
section updating `extraTxnState`.

Epic: none
Fixes: cockroachdb#119284

Release note: None
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Feb 26, 2024
We will only attempt to sample the transaction once at the
start of its execution.

Additional changes:
- Move the sampling decision for transactions to be closer to section
updating `extraTxnState` and modifies
- Use the concrete statement types in `shouldForceLogStatement` to
prevent force logging on BEGIN and COMMIT statements.

Epic: none
Fixes: cockroachdb#119284

Release note: None
craig bot pushed a commit that referenced this issue Mar 5, 2024
119481: telemetry: don't resample transaction for telemetry on restart r=xinhaoz a=xinhaoz

### telemetry: don't resample transaction for telemetry on restart

We will only attempt to sample the transaction once at the
start of its execution.

Additional changes:
- Move the sampling decision for transactions to be closer to section
updating `extraTxnState`
- Use the concrete statement types in `shouldForceLogStatement` to
prevent force logging on BEGIN and COMMIT statements.

Epic: none
Fixes: #119284

Release note: None

### eventpbgen: rename Nullable property to NotNullable

Let's rename the `Nullable` field to `NotNullable` for the
`fieldInfo` struct used in generating json encoding functions
for protobuf definitons. Note this field currently only applies to
`nestedMessage` types.

Epic: none

Release note: None

119893: lint: various fixes and improvements r=rail a=rickystewart

1. Use code generation to remove the `lint` build tag for running lints.
   On the Bazel side, you're not likely to accidentally run these tests
   and build tags just slow things down.
2. Fix the script for the nightly lint to exit with the correct status
   code and generate code before running.
3. Fix some lint errors in `lint_test.go` itself.

Epic: none
Release note: None

119951: sessiondatapb: fix build failures under `race` r=rail a=rickystewart

Regressed in #118546

Closes #119950

Epic: none
Release note: None

Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
@craig craig bot closed this as completed in 6d1d6ee Mar 5, 2024
blathers-crl bot pushed a commit that referenced this issue Mar 5, 2024
We will only attempt to sample the transaction once at the
start of its execution.

Additional changes:
- Move the sampling decision for transactions to be closer to section
updating `extraTxnState`.
- Use the concrete statement types in `shouldForceLogStatement` to
prevent force logging on BEGIN and COMMIT statements.

Epic: none
Fixes: #119284

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant