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

mock: unify expected span checking #3096

Closed
wants to merge 1 commit into from
Closed

Conversation

hds
Copy link
Contributor

@hds hds commented Oct 3, 2024

Motivation

Previously there were two separate implementations which would check an
ExpectedSpan against and actual span. One on the ExpectedSpan struct
itself, which took a SpanState as used by the MockCollector and
another on the MockSubscriber which took a SpanRef (from
tracing-subscriber).

In reality, both of these checks needed a span::Id and a Metadata to
check, but the structure was also somewhat different, with the
MockSubscriber span check giving better error output.

Solution

This change combines the two checks into the one on ExpectedSpan,
which is now generic over an ActualSpan implementation, which has been
provided for SpanState, SpanRef, and also span::Id (for the case
where no Metadata is available).

The better error output from MockSubscriber has been integrated into
that check.

@hds hds requested review from hawkw, davidbarsky and a team as code owners October 3, 2024 13:05
Previously there were two separate implementations which would check an
`ExpectedSpan` against and actual span. One on the `ExpectedSpan` struct
itself, which took a `SpanState` as used by the `MockCollector` and
another on the `MockSubscriber` which took a `SpanRef` (from
`tracing-subscriber`).

In reality, both of these checks needed a `span::Id` and a `Metadata` to
check, but the structure was also somewhat different, with the
`MockSubscriber` span check giving better error output.

This change combines the two checks into the one on `ExpectedSpan`,
which is now generic over an `ActualSpan` implementation, which has been
provided for `SpanState`, `SpanRef`, and also `span::Id` (for the case
where no `Metadata` is available).

The better error output from `MockSubscriber` has been integrated into
that check.
@hds hds force-pushed the hds/mock-expected-span-check branch from 935bbe0 to 1da4c31 Compare October 3, 2024 13:35
@hds
Copy link
Contributor Author

hds commented Oct 4, 2024

This change has been merged into #3098 and modified significantly.

@hds hds closed this Oct 4, 2024
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.

1 participant