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

Rationalise interfaces for Spans and Transactions #2656

Closed
jamescrosswell opened this issue Sep 26, 2023 · 1 comment
Closed

Rationalise interfaces for Spans and Transactions #2656

jamescrosswell opened this issue Sep 26, 2023 · 1 comment
Milestone

Comments

@jamescrosswell
Copy link
Collaborator

jamescrosswell commented Sep 26, 2023

Version 3.x.x Interfaces

erDiagram
    NoOpSpan ||--|| ISpan : implements

    SpanTracer ||--|| ISpan : implements

    ISpan ||--|| ISpanData : implements
    ISpan {
        string Description
        string Operation
        SpanStatus Status
        ISpan StartChild()
        void Finish()
    }

    Span ||--|| ISpanData : implements

    ISpanData ||--|| ISpanContext : implements
    ISpanData ||--|| IHasTags : implements
    ISpanData ||--|| IHasExtra : implements

    ISpanContext ||--|| ITraceContext : implements

    NoOpTransaction ||--|| ITransaction : implements

    TransactionTracer ||--|| ISpan : implements
    TransactionTracer ||--|| ITransaction : implements
    TransactionTracer ||--|| IHasDistribution : implements
    TransactionTracer ||--|| IHasTransactionNameSource : implements
    TransactionTracer ||--|| IHasMeasurements : implements

    ITransaction ||--|| ITransactionData : implements
    ITransaction ||--|| ISpan : implements
    ITransaction {
        string Name
        bool IsParentSampled
        IReadOnlyCollection Spans
        ISpan GetLastActiveSpan()
    }

    Transaction ||--|| ITransactionData : implements
    Transaction ||--|| IHasDistribution : implements
    Transaction ||--|| IHasTransactionNameSource : implements
    Transaction ||--|| IHasMeasurements : implements

    ITransactionData ||--||  ISpanData : implements
    ITransactionData ||--|| ITransactionContext : implements
    ITransactionData ||--|| IEventLike : implements

    ITransactionContext ||--|| ISpanContext : implements

    IEventLike ||--|| IHasBreadcrumbs : implements
    IEventLike ||--|| IHasTags : implements
    IEventLike ||--|| IHasExtra : implements

    TransactionContext ||--|| SpanContext : implements
    TransactionContext ||--|| ITransactionContext : implements

    SpanContext ||--|| ITraceContext : implements
Loading

Opportunities to simplify

Remove interfaces that were planned for removal in v4.0

Various interfaces were added because we were unable to add properties to existing interfaces... they were never intended to be kept long term:

Remove empty interfaces

We could remove interfaces that don't have any properties or methods (unless they serve some useful function as marker interfaces).

Remove interfaces with only one implementation

Merge interfaces that share all the same implementors

That still leaves us with a slightly simpler problem to solve however:

erDiagram
    NoOpSpan ||--|| ISpan : implements

    SpanTracer ||--|| ISpan : implements
    SpanTracer {
        bool IsSentryRequest
        TransactionTracer Transaction
        void Unfinish()
    }


    ISpan ||--|| ISpanData : implements
    ISpan {
        string Description
        string Operation
        SpanStatus Status
        ISpan StartChild()
        void Finish()
    }

    Span ||--|| ISpanData : implements

    ISpanData ||--|| ITraceContext : implements
    ISpanData ||--|| IHasTags : implements
    ISpanData ||--|| IHasExtra : implements
    ISpanData {
        DateTimeOffset StartTimestamp
        DateTimeOffset EndTimestamp
        bool IsFinished
        SentryTraceHeader GetTraceHeader()
    }

    NoOpTransaction ||--|| ITransaction : implements

    TransactionTracer ||--|| ISpan : implements
    TransactionTracer ||--|| ITransaction : implements
    TransactionTracer ||--|| ITransactionData : implements
    TransactionTracer ||--|| ISpan : implements
    TransactionTracer {
        bool IsSentryRequest
        double SampleRate
        IReadOnlyCollection Spans
        DynamicSamplingContext DynamicSamplingContext
        ITransactionProfiler TransactionProfiler
        void AddChildSpan()
        ISpan GetLastActiveSpan()
    }

    ITransaction ||--|| ITransactionData : implements
    ITransaction ||--|| ISpan : implements
    ITransaction {
        string Name
        bool IsParentSampled
        IReadOnlyCollection Spans
        ISpan GetLastActiveSpan()
    }

    Transaction ||--|| ITransactionData : implements
    Transaction {
        SentryId EventId
        double SampleRate
        DynamicSamplingContext DynamicSamplingContext
        ITransactionProfiler TransactionProfiler
        IReadOnlyCollection Spans
        void Redact()
        void WriteTo()
        Transaction FromJson()
    }

    ITransactionData ||--||  ISpanData : implements
    ITransactionData ||--|| ITransactionContext : implements
    ITransactionData ||--|| IEventLike : implements

    ITransactionContext ||--|| ITraceContext : implements

    IEventLike ||--|| IHasTags : implements
    IEventLike ||--|| IHasExtra : implements

    TransactionContext ||--|| SpanContext : implements
    TransactionContext ||--|| ITransactionContext : implements

    SpanContext ||--|| ITraceContext : implements
Loading
@jamescrosswell jamescrosswell added this to the 4.0.0 milestone Sep 26, 2023
@bruno-garcia
Copy link
Member

Great stuff! Thanks for breaking this up! I took a look at some of the PRs, I think 1 left. @bitsandfoxes had the idea of adding [Obsolete] right now, on the 3.x, to all the stuff we're removing in 4.0.0. If not a lot of work, could be helpful? That said I don't expect anyone to be using these types directly unless some IDE suggested using a less specific type on some function argument or something. Very unlikely

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

No branches or pull requests

2 participants