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

Feature request: Add sampling flag to MDC #755

Open
jamesmoessis opened this issue Jun 18, 2024 · 3 comments
Open

Feature request: Add sampling flag to MDC #755

jamesmoessis opened this issue Jun 18, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@jamesmoessis
Copy link
Contributor

jamesmoessis commented Jun 18, 2024

The Slf4jEventListener in this repo is what adds the trace ID and the span ID to the logs.

It's useful to add the sampling flag to the logs. An example use case being a developer looking at their logs and being able to filter the logs which would have a trace present in their tracing system.

This could be added to Slf4jEventListener as an optional field.

Would love the maintainer's thoughts. Would be happy to implement myself.

I'm coming from the perspective of the otel bridge but of course I assume the same thing could be implemented for brave.

@bams22
Copy link

bams22 commented Aug 5, 2024

It looks like a good change. I'm sure many people will appreciate it. Why not add this change made by @jamesmoessis without waiting for the comprehensive solution that @jamesmoessis mentioned in his PR?

@jamesmoessis
Copy link
Contributor Author

@bams22, Spoke to the maintainer @jonatan-ivanov didn't want the change to go in because they are working on an adjacent feature set that solves this for more than just sampled flag

@jamesmoessis
Copy link
Contributor Author

jamesmoessis commented Aug 6, 2024

@bams22 If you want to add this into your own project, register a bean that does it. This will add onto existing event listeners.

I can share with you my one:

/**
 * Reacts to tracing context changes.
 * Annotates a boolean on the MDC to indicate whether the trace context is sampled.
 * Essentially a {@link io.micrometer.tracing.otel.bridge.Slf4JEventListener} but
 * for sampling flag instead of traceID and spanID.
 */
@Component
final class MdcSampledEventListener implements EventListener {
    private final String sampledKey;

    /**
     * @param sampledKey the key to be put on the MDC
     */
    MdcSampledEventListener(String sampledKey) {
        this.sampledKey = sampledKey;
    }

    private void onScopeAttached(EventPublishingContextWrapper.ScopeAttachedEvent event) {
        Span span = event.getSpan();
        if (span != null) {
            MDC.put(this.sampledKey, Boolean.toString(span.getSpanContext().isSampled()));
        }
    }

    private void onScopeRestored(EventPublishingContextWrapper.ScopeRestoredEvent event) {
        Span span = event.getSpan();
        if (span != null) {
            MDC.put(this.sampledKey, Boolean.toString(span.getSpanContext().isSampled()));
        }

    }

    private void onScopeClosed(EventPublishingContextWrapper.ScopeClosedEvent event) {
        MDC.remove(this.sampledKey);
    }

    public void onEvent(Object event) {
        if (event instanceof EventPublishingContextWrapper.ScopeAttachedEvent) {
            this.onScopeAttached((EventPublishingContextWrapper.ScopeAttachedEvent) event);
        } else if (event instanceof EventPublishingContextWrapper.ScopeClosedEvent) {
            this.onScopeClosed((EventPublishingContextWrapper.ScopeClosedEvent) event);
        } else if (event instanceof EventPublishingContextWrapper.ScopeRestoredEvent) {
            this.onScopeRestored((EventPublishingContextWrapper.ScopeRestoredEvent) event);
        }
    }
}

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

Successfully merging a pull request may close this issue.

3 participants