observability: tracing gum, automatically cross ref traceID #5079

drahnr · 2022-03-10T16:16:45Z

Follow up to #5067

Will provide a tracing::{warn,error,debug,trace,info} compatible set of macros, but tracks any occurrences of identifiers or aliases that match candidate_hash and derives a traceID = ... from those and provides that to the tracing::event!($args) as separate key value pair. This being in place should be sufficient for grafana to cross link logs to zipkin spans.

Bottom line using gum::warn! instead of tracing::warn! should be used to get implicit cross ref based on the candidate_hash derived trace identifier in the grafana tempo ui.

CC @lazam

rphmeier · 2022-03-10T17:35:51Z

Have we already created or do we have plans to create per-candidate spans that follow it through all stages (collation, backing, approval, disputes)? Or is this cross-referencing orthogonal?

My concern is orphaned traces, where we have candidates logged in approval/disputes and either get linked into the void or to some backing span that is unrelated.

drahnr · 2022-03-10T22:13:29Z

The linking meant here is between logs in Loki and spans in tempo, such that the grafana UI works properly.

We don't have spans beyond subsystems as far as I can remember, but as long as they all reference the same traced derived from a candidate hash, we should will be able to track them across stages by tag and stage annotations for the individual spans. Having orphans hence shouldn't be that big of an issue, at least I don't see how it would impair the debugging flow.

rphmeier · 2022-03-10T22:15:59Z

The linking meant here is between logs in Loki and spans in tempo, such that the grafana UI works properly.
We don't have spans beyond subsystems as far as I can remember.

Approval and disputes are subsystems. and my point is that there are few/no spans there, but many log lines referring to candidate hash. Unclear what should happen if/when we add approval/disputes spans.

drahnr · 2022-03-10T22:18:12Z

The cross linking is also bounded by time, which I did not mention, so this should further restrict the amount of query results

ordian

It's not clear if candidate_hash based traceID is the only one we'll be interested in. Looks good otherwise.

Cargo.lock

node/gum/proc-macro/src/types.rs

node/gum/proc-macro/Cargo.toml

node/gum/README.md

node/gum/proc-macro/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

drahnr · 2022-03-15T09:35:44Z

It's not clear if candidate_hash based traceID is the only one we'll be interested in. Looks good otherwise.

For the time being, we'll stick to candidate hash, and check if we can have multiple identically keyed tags on a span, it has to be traceID for the log line after all.

sandreim

I ❤️ it - adds no extra LoCs. Otherwise, I think we should add a basic integration test in this PR or a followup.

node/gum/README.md

node/gum/proc-macro/src/lib.rs

sandreim · 2022-03-15T09:40:12Z

node/gum/proc-macro/src/lib.rs

+	let Args { target, comma, mut values, fmt } = args;
+
+	// find a value or alias called `candidate_hash`.
+	let maybe_candidate_hash = values.iter_mut().find(|value| value.as_ident() == "candidate_hash");


As a followup PR: Is it possible to remove the hardcoding here to derive the trace_id from something specified by the user ?

Technically yes, but that would add additional syntax. Open to discuss something along the lines of foo @ traceID though, out of scope for this PR though

sandreim · 2022-03-15T09:46:45Z

node/gum/proc-macro/src/lib.rs

+			}
+		})
+	} else {
+		Ok(quote! {


Would a fall back to relay_parent add value ?

Not sure yet, it could also be more confusing since all of a sudden you have mix of relay chain and parachain hashes in the same view.

I think relay_parent might be another tracing id in conjunction with candidate_hash as it is available in more places than candidate_hash. But I also think that these IDs should be kept separately, as fallback would only mess things up.

Yeah, that would mess things up, having a secondary trace id is what makes sense here. Falling back to a secondary trace id is what I really meant.

Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com>

drahnr · 2022-03-15T11:05:08Z

bot merge

github-actions bot added the A0-please_review Pull request needs code review. label Mar 10, 2022

drahnr self-assigned this Mar 10, 2022

drahnr added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Mar 10, 2022

drahnr requested a review from sandreim March 10, 2022 16:42

drahnr changed the title ~~tracing gum, actually cross ref traceID~~ WIP: tracing gum, actually cross ref traceID Mar 10, 2022

drahnr marked this pull request as draft March 10, 2022 16:42

drahnr changed the title ~~WIP: tracing gum, actually cross ref traceID~~ WIP: tracing gum, automatically cross ref traceID Mar 10, 2022

drahnr force-pushed the bernhard-tracing-gum branch 2 times, most recently from 7c6ca63 to 49cddbf Compare March 11, 2022 09:56

drahnr marked this pull request as ready for review March 11, 2022 15:27

drahnr added this to the v0.9.19 milestone Mar 11, 2022

drahnr changed the title ~~WIP: tracing gum, automatically cross ref traceID~~ observability: tracing gum, automatically cross ref traceID Mar 11, 2022

drahnr requested a review from ordian March 11, 2022 15:48

ordian reviewed Mar 11, 2022

View reviewed changes

Cargo.lock Outdated Show resolved Hide resolved

node/gum/proc-macro/src/types.rs Outdated Show resolved Hide resolved

node/gum/proc-macro/Cargo.toml Outdated Show resolved Hide resolved

node/gum/README.md Show resolved Hide resolved

bkchr reviewed Mar 11, 2022

View reviewed changes

node/gum/proc-macro/src/lib.rs Outdated Show resolved Hide resolved

drahnr and others added 10 commits March 14, 2022 14:51

add some gum

720355a

bump expander

f7ee9ae

gum

6657f6e

fix all remaining issues

2384b07

last fixup

2363b91

Update node/gum/proc-macro/src/lib.rs

9bb0441

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

change

df3f753

netowrk

9b580ab

fixins

a450ca0

chore

3cb1254

drahnr added 7 commits March 14, 2022 14:53

allow optional fmt str + args, prep for expr as kv field

6ea8178

tracing -> gum rename fallout

ec437c2

restrict further

0130947

allow multiple levels of field accesses

397a680

another round of docs and a slip of the pen

81937b3

update ADR

68e045f

fixup lock fiel

cdab19e

drahnr force-pushed the bernhard-tracing-gum branch from dbc133f to cdab19e Compare March 14, 2022 13:54

drahnr added 5 commits March 14, 2022 15:13

use target: instead of target=

0cb6386

minors

a6f5bbd

fix

8edf4eb

chore

f626141

Merge remote-tracking branch 'origin/master' into bernhard-tracing-gum

638d63b

sandreim approved these changes Mar 15, 2022

View reviewed changes

Update node/gum/README.md

42c7365

Co-authored-by: Andrei Sandu <54316454+sandreim@users.noreply.github.com>

paritytech-processbot bot merged commit 6cafab2 into master Mar 15, 2022

paritytech-processbot bot deleted the bernhard-tracing-gum branch March 15, 2022 11:05

Garandor mentioned this pull request Mar 15, 2022

Distributed Request Tracing Client <-> Signer <-> Node Runtime Manta-Network/Manta#448

Closed

drahnr mentioned this pull request Mar 17, 2022

TraceID is not provided for spans and traces #5045

Closed

coderobe mentioned this pull request Apr 8, 2022

Polkadot v0.9.19 Release checklist #5288

Closed

16 tasks

drahnr mentioned this pull request Sep 27, 2022

Member request polkadot-fellows/seeding#15

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

observability: tracing gum, automatically cross ref traceID #5079

observability: tracing gum, automatically cross ref traceID #5079

drahnr commented Mar 10, 2022 •

edited

Loading

rphmeier commented Mar 10, 2022 •

edited

Loading

drahnr commented Mar 10, 2022 •

edited

Loading

rphmeier commented Mar 10, 2022 •

edited

Loading

drahnr commented Mar 10, 2022 •

edited

Loading

ordian left a comment

drahnr commented Mar 15, 2022 •

edited

Loading

sandreim left a comment

sandreim Mar 15, 2022

drahnr Mar 15, 2022 •

edited

Loading

sandreim Mar 15, 2022 •

edited

Loading

drahnr Mar 15, 2022

vstakhov Mar 15, 2022

sandreim Mar 15, 2022 •

edited

Loading

drahnr commented Mar 15, 2022

observability: tracing gum, automatically cross ref traceID #5079

observability: tracing gum, automatically cross ref traceID #5079

Conversation

drahnr commented Mar 10, 2022 • edited Loading

rphmeier commented Mar 10, 2022 • edited Loading

drahnr commented Mar 10, 2022 • edited Loading

rphmeier commented Mar 10, 2022 • edited Loading

drahnr commented Mar 10, 2022 • edited Loading

ordian left a comment

Choose a reason for hiding this comment

drahnr commented Mar 15, 2022 • edited Loading

sandreim left a comment

Choose a reason for hiding this comment

sandreim Mar 15, 2022

Choose a reason for hiding this comment

drahnr Mar 15, 2022 • edited Loading

Choose a reason for hiding this comment

sandreim Mar 15, 2022 • edited Loading

Choose a reason for hiding this comment

drahnr Mar 15, 2022

Choose a reason for hiding this comment

vstakhov Mar 15, 2022

Choose a reason for hiding this comment

sandreim Mar 15, 2022 • edited Loading

Choose a reason for hiding this comment

drahnr commented Mar 15, 2022

drahnr commented Mar 10, 2022 •

edited

Loading

rphmeier commented Mar 10, 2022 •

edited

Loading

drahnr commented Mar 10, 2022 •

edited

Loading

rphmeier commented Mar 10, 2022 •

edited

Loading

drahnr commented Mar 10, 2022 •

edited

Loading

drahnr commented Mar 15, 2022 •

edited

Loading

drahnr Mar 15, 2022 •

edited

Loading

sandreim Mar 15, 2022 •

edited

Loading

sandreim Mar 15, 2022 •

edited

Loading