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

feat(otelzap): add dynamic field value resolution #82

Closed
wants to merge 1 commit into from

Conversation

goober
Copy link

@goober goober commented Oct 19, 2022

This PR aims to add a possibility to dynamically resolve fields from the provided context and fix #59.

Our use case is that Datadog requires us to provide both the spanId and the traceId to the logged message in the keys dd.span_id and dd.trace_id. In addition, Datadog expects the trace information to be in another format for traces and logs to be correlated. This change will make it possible for us to configure this transformation in the logger itself instead of providing the transformation in every log message.

Below is a sample implementation for Datadog with this change that will print the converted trace information:
ERROR example/main.go:29 hello from zap {"dd.trace_id": "2730366003796952559", "dd.span_id": "2730366003796952559"}

func main() {
    l, err := zap.NewDevelopment()
    if err != nil {
        panic(err)
    }
    logger = otelzap.New(l, WithDatadogFields())
    
    ctx, span := tracer.Start(context.Background(), "root")
    defer span.End()

    logger.Ctx(ctx).Error("hello from zap")
}

func WithDatadogFields() otelzap.Option {
	convertTraceId := func(id string) string {
		if len(id) < 16 {
			return ""
		}
		if len(id) > 16 {
			id = id[16:]
		}
		intValue, err := strconv.ParseUint(id, 16, 64)
		if err != nil {
			return ""
		}
		return strconv.FormatUint(intValue, 10)
	}

	return otelzap.WithDynamicFields(func(ctx context.Context) []zap.Field {
		fields := make([]zap.Field, 0)
		span := trace.SpanFromContext(ctx)
		if span.IsRecording() {
			fields = append(fields, zap.String("dd.trace_id", convertTraceId(span.SpanContext().TraceID().String())))
			fields = append(fields, zap.String("dd.span_id", convertTraceId(span.SpanContext().SpanID().String())))
		}
		return fields
	})
}

@goober
Copy link
Author

goober commented Nov 28, 2022

@vmihailenco I just want to reach out to hear your opinion related to this PR. Is it a functionality that you see is feasible to be included in this project or is it more suitable to wrap this library and add the dynamic fields as part of the wrapped implementation?

I have no strong opinion, even though it could be a nice developer experience to not have to wrap the library, but instead give the implementor the freedom to adjust it after their need without adding an extra burden on the maintainers to keep adding support for each possible use case when they arise.

@vmihailenco
Copy link
Member

The package now uses Otel Logs API so there is no need for custom trace_id/span_id fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add zap fields and setting attribututes to trace in context
2 participants