improve span order

[zeitlinger]

• sort spans by start time (parents before children as tiebreaker) to avoid common causes for flaky tests

[codecov]

Codecov Report

Base: 91.06% // Head: 91.22% // Increases project coverage by +0.16% 🎉

Coverage data is based on head (618e5d2) compared to base (b091536).
Patch coverage: 77.77% of modified lines in pull request are covered.

❗ Current head 618e5d2 differs from pull request most recent head b8a7194. Consider uploading reports for the commit b8a7194 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5026      +/-   ##
============================================
+ Coverage     91.06%   91.22%   +0.16%     
+ Complexity     4886     4884       -2     
============================================
  Files           553      553              
  Lines         14465    14433      -32     
  Branches       1387     1376      -11     
============================================
- Hits          13172    13166       -6     
+ Misses          896      883      -13     
+ Partials        397      384      -13     

Impacted Files	Coverage Δ
.../io/opentelemetry/sdk/testing/trace/TraceUtil.java	50.00% <50.00%> (ø)
...pentelemetry/sdk/testing/assertj/TracesAssert.java	92.00% <100.00%> (+4.50%)	⬆️
...try/sdk/testing/junit5/OpenTelemetryExtension.java	100.00% <100.00%> (ø)
.../logging/internal/LoggingSpanExporterProvider.java	66.66% <0.00%> (-33.34%)	⬇️
...ogging/internal/LoggingMetricExporterProvider.java	66.66% <0.00%> (-33.34%)	⬇️
...ing/internal/LoggingLogRecordExporterProvider.java	66.66% <0.00%> (-33.34%)	⬇️
.../sdk/autoconfigure/MeterProviderConfiguration.java	77.27% <0.00%> (-22.73%)	⬇️
...emetry/sdk/metrics/internal/debug/DebugConfig.java	90.00% <0.00%> (-10.00%)	⬇️
...er/zipkin/internal/ZipkinSpanExporterProvider.java	90.00% <0.00%> (-10.00%)	⬇️
...ry/exporter/internal/marshal/CodedInputStream.java	57.89% <0.00%> (-5.27%)	⬇️
... and 75 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[zeitlinger]

I didn't change anything there, just modified the comparator. Apart from that, this is a typical implicit guarantee in java, because backwards compatibility is honored.

…

On Mon, Dec 12, 2022, 19:19 jason plumb ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In sdk/testing/src/main/java/io/opentelemetry/sdk/testing/assertj/TracesAssert.java <#5026 (comment)> : > @@ -21,6 +25,22 @@ extends AbstractIterableAssert< TracesAssert, List<List<SpanData>>, List<SpanData>, TraceAssert> { + /** + * Returns an assertion for a list of traces. The provided spans will be grouped into traces by + * their trace ID. + */ + public static TracesAssert assertThat(List<SpanData> spanData) { + Map<String, List<SpanData>> traces = + spanData.stream() + .collect( + Collectors.groupingBy( + SpanData::getTraceId, LinkedHashMap::new, Collectors.toList())); + for (List<SpanData> trace : traces.values()) { + trace.sort(SPAN_DATA_COMPARATOR); Nitpick: I think there's no guarantee that the List returned by Collectors.toList() is mutable, and Collectors.toCollection(supplier) can be used as a work-around. — Reply to this email directly, view it on GitHub <#5026 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVTR453WOZZRRYTIJAD3L3WM5UERANCNFSM6AAAAAASYJH2AQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>