Add spans around (pre)certificate signing #7707
Conversation
beautifulentropy
requested review from
a team and
jsha
and removed request for
a team
| @@ -272,6 +277,7 @@ func NewCertificateAuthorityImpl( | |||
| keyPolicy: keyPolicy, | |||
| log: logger, | |||
| metrics: metrics, | |||
| tracer: otel.GetTracerProvider().Tracer("github.com/letsencrypt/boulder/ca"), | |||
There was a problem hiding this comment.
https://pkg.go.dev/go.opentelemetry.io/otel/trace#TracerProvider
// The passed name needs to uniquely identify instrumentation code.
// Therefore, it is recommended that name is the Go package name of the
// library providing instrumentation (note: not the code being
// instrumented).
This is a little confusing - does "instrumentation code" here refer to otel? Surely not or it would be defaulted. I guess in our case, github.com/letsencrypt/boulder/ca is both the library providing instrumentation and the code being instrumented. 🤷🏻
There was a problem hiding this comment.
I think this distinction is only important in the case of, eg, a middleware adding instrumentation to something else. I think in this case it is instrumenting itself. I did a bit of a survey on Github to see what other projects are doing, and it does look like this is consistent with other packages instrumenting themselves.
This adds custom spans around one of the more important parts of the issuance stack - actually signing the (pre)certificates. We only have automatic tracing right now, so this is just a small step towards adding more customization there.
One specific note: I didn't include the regID in the span attributes, though it is in the nearby log lines. I think that's something we likely want to handle holistically (eg, via baggage propagation) rather than one-off in manual spans like this.