Skip to content

Conversation

@iamdanfox
Copy link
Contributor

@iamdanfox iamdanfox commented Aug 23, 2019

Before this PR

I sometimes find it hard to intuitively imagine what spans look like, especially when we're talking about async stuff. We also don't have a good way of testing spans in our libraries (e.g. conjure-java-runtime).

After this PR

==COMMIT_MSG==
Tests can now be annotated @TestTracing to set up snapshot testing.
==COMMIT_MSG==

Possible downsides / known limitations

  • we don't currently serialize the 'type' (i.e. LOCAL / SERVER_OUTGOING), so can't make assertions based on it
  • if users call Tracer.setSampler part way through the tests (which WC does), they'll get incomplete info in the tests
  • running JUnit5 tests concurrently will result in very strange looking HTML reports
  • when snapshotting is enabled, renaming test classes/methods could result in extraneous trace files left around
  • we're not actually using the SLS trace.1 format

@iamdanfox iamdanfox requested a review from a team as a code owner August 23, 2019 16:07
@palantir palantir deleted a comment from changelog-app bot Aug 23, 2019
@iamdanfox iamdanfox changed the title [test-only] HTML span observer Test-utils to visualize spans Aug 23, 2019
@iamdanfox
Copy link
Contributor Author

iamdanfox commented Aug 23, 2019

I've tried this on c-j-r which is a bit weird because many tests don't actually neatly close a trace. I think it gives reasonable output.

@ferozco ferozco force-pushed the dfox/render-spans branch from 3134b85 to dd7fba6 Compare August 28, 2019 13:54
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

final class TestTracingExtension implements BeforeEachCallback, AfterEachCallback {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why BeforeEachCallback rather than BeforeTestExecution?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like BeforeTestExecution was causing the spans within the setup method to also be captured

@ferozco ferozco merged commit 789446d into develop Aug 29, 2019
@ferozco ferozco deleted the dfox/render-spans branch August 29, 2019 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants