otelriver: add extra attributes

[krhubert]

Hey,

I saw some job attributes in otelriver source code and I want to share my implemenatation as a starting point for discussion about extra attributes.

    spanOpts := []trace.SpanStartOption{
        trace.WithAttributes(
            attribute.Int64("river.job.id", job.ID),
            attribute.Int("river.job.attempt", job.Attempt),
            attribute.Int64("river.job.created_at", job.CreatedAt.Unix()),
            attribute.String("river.job.kind", job.Kind),
            attribute.String("river.job.queue", job.Queue),
            attribute.Int64("river.job.scheduled_at", job.ScheduledAt.Unix()),
            attribute.Int("river.job.priority", job.Priority),
            attribute.StringSlice("river.job.tags", job.Tags),
        ),
        trace.WithSpanKind(trace.SpanKindConsumer),
    }

    ctx, span := wm.cfg.Tracer.Start(ctx, job.Kind, spanOpts...)
    defer span.End()

    err := doInner(ctx)
    if err != nil {
        if ok := errors.Is(err, &river.JobSnoozeError{}); ok {
            // Snooze errors are expected and should not be recorded as errors.
            // but rather as informational events.
            //
            // do not set the set status to error, but keep the unset.
            // This way the success/error ratio is not affected by snoozed jobs.
            span.SetAttributes(attribute.Bool("river.job.snoozed", true))
        } else {
            // otherwise, record the error as an error event.
            cancelErr := &river.JobCancelError{}
            if ok := errors.As(err, &cancelErr); ok {
                // try to unwrap the error to get the original error.
                //
                // TODO: think how we want to treat cancelled jobs.
                // On one hand, they are expected and should not be recorded as errors,
                // but on the other hand, they are not successful jobs.
                // For now, we just record them as error.
                span.SetAttributes(attribute.Bool("river.job.cancelled", true))
                err = cancelErr.Unwrap()
            }

            span.SetStatus(codes.Error, err.Error())
            span.RecordError(err)
        }
    } else {
        span.SetStatus(codes.Ok, "")
    }

Things that are different

1. add SpanKindConsumer

2. Extra span attributes:

• "river.job.id"
• "river.job.created_at"
• "river.job.scheduled_at"
• "river.job.priority"
• "river.job.tags"

I think all of them might be helpful for logging, debugging and the dashboard. Maybe the attributes list can be configurable? I know this is not a common pattern in otel (configurable list of attrs), but why not give it a shot?

3. special case for handling cancelled and snoozed jobs, which you can find more in the implementation I shared.

Thanks!

[brandur]

Yeah I like it. As far as metrics are concerned, I've found a few things in the spec that seem to suggest that there possibly should be an upper bound to cardinality [1], which would be a problem for open ended sets like IDs or timestamps, but I suppose that it shouldn't be a problem for spans.

Do you know if there's a clear preference that timestamps should be Unix integers like you have in the code above? I'd usually prefer to make them RFC3339 instead because that's readable on sight by a human without needing a computer's help.

[1] https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#cardinality-limits

[krhubert]

Do you know if there's a clear preference that timestamps should be Unix integers like you have in the code above? I'd usually prefer to make them RFC3339 instead because that's readable on sight by a human without needing a computer's help.

I think it should be RFC3339. I created this as unix timestamp, but this is not a standard approach.

which would be a problem for open ended sets like IDs or timestamps, but I suppose that it shouldn't be a problem for spans.

Yeah, good point. Maybe ids and timestamps are not needed after all. Logs can contain this information, and they can be correlated with spans and traces.