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

kv,sql: test propagation of trace spans on error #55594

Open
tbg opened this issue Oct 15, 2020 · 0 comments
Open

kv,sql: test propagation of trace spans on error #55594

tbg opened this issue Oct 15, 2020 · 0 comments
Labels
A-kv-observability A-sql-observability Related to observability of the SQL layer A-tracing Relating to tracing in CockroachDB. C-investigation Further steps needed to qualify. C-label will change. T-kv KV Team

Comments

@tbg
Copy link
Member

tbg commented Oct 15, 2020

The KV layer does (or at least should) propagate trace recordings on errors. We the *BatchResponse as a carrier, which also carries any *Error.

if rec := tracing.GetRecording(grpcSpan); rec != nil {
br.CollectedSpans = append(br.CollectedSpans, rec...)
}

The SQL layer uses RemoteProducerMetadata (or ProducerMetadata? I'm not quite sure):

rpm.Value = &RemoteProducerMetadata_TraceData_{
TraceData: &RemoteProducerMetadata_TraceData{
CollectedSpans: meta.TraceData,
},
}

It hopefully also does propagate errors.

While introducing always-on tracing, we want to spend some time making sure that errors in fact do correctly propagate through traces at all times via the introduction of suitable tests where they do not yet exist.

Jira issue: CRDB-3655

@tbg tbg added A-kv-observability A-sql-observability Related to observability of the SQL layer C-investigation Further steps needed to qualify. C-label will change. labels Oct 15, 2020
@tbg tbg changed the title kv,sql: fix propgation of trace spans on error kv,sql: test propagation of trace spans on error Oct 15, 2020
@tbg tbg added this to MH (Irfan Raphael Sam Tobias) in KV 21.1 Stability Period Oct 15, 2020
@tbg tbg added the A-tracing Relating to tracing in CockroachDB. label Jan 20, 2021
@tbg tbg moved this from MH (Irfan Raphael Tobias) to To be considered in KV 21.1 Stability Period Jan 20, 2021
@tbg tbg moved this from To be considered to MH (Irfan Raphael Tobias) in KV 21.1 Stability Period Jan 20, 2021
@tbg tbg moved this from MH (Irfan Raphael Tobias) to NH in KV 21.1 Stability Period Feb 10, 2021
@tbg tbg moved this from NH to Unlikely/Nope in KV 21.1 Stability Period Mar 4, 2021
@tbg tbg removed this from Unlikely/Nope in KV 21.1 Stability Period Mar 9, 2021
@tbg tbg added this to Incoming in KV via automation Mar 9, 2021
@jlinder jlinder added the T-kv KV Team label Jun 16, 2021
@mwang1026 mwang1026 removed the T-kv KV Team label Aug 20, 2021
@mwang1026 mwang1026 removed this from Incoming in KV Aug 20, 2021
@mwang1026 mwang1026 added this to To do in Observability Infrastructure via automation Aug 20, 2021
@exalate-issue-sync exalate-issue-sync bot added T-kv KV Team and removed T-observability-inf labels Apr 11, 2023
@blathers-crl blathers-crl bot added this to Incoming in KV Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv-observability A-sql-observability Related to observability of the SQL layer A-tracing Relating to tracing in CockroachDB. C-investigation Further steps needed to qualify. C-label will change. T-kv KV Team
Projects
KV
Incoming
Development

No branches or pull requests

3 participants