O11Y-1504: wo11y-dart tracer.trace spans don't end until the parent s…

…pan ends - Consolidating API.
Workiva · Mar 10, 2022 · 38aef43 · 38aef43
1 parent f83c6f2
commit 38aef43
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 49 deletions.
diff --git a/lib/src/api/trace/noop_tracer.dart b/lib/src/api/trace/noop_tracer.dart
@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import '../../../api.dart' as api;
 import '../../../sdk.dart' as sdk;
 
@@ -14,13 +16,8 @@ class NoopTracer implements api.Tracer {
   }
 
   @override
-  Future<R> traceAsync<R>(String name, Future<R> Function() fn,
+  FutureOr<R> trace<R>(String name, FutureOr<R> Function() fn,
       {api.Context context}) async {
     return await (context ?? api.Context.current).execute(fn);
   }
-
-  @override
-  R traceSync<R>(String name, R Function() fn, {api.Context context}) {
-    return (context ?? api.Context.current).execute(fn);
-  }
 }
diff --git a/lib/src/api/trace/tracer.dart b/lib/src/api/trace/tracer.dart
@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import '../../../api.dart' as api;
 
 /// An interface for creating [api.Span]s and propagating context in-process.
@@ -10,12 +12,8 @@ abstract class Tracer {
   api.Span startSpan(String name,
       {api.Context context, api.Attributes attributes});
 
-  /// Records a span of the given [name] for the given synchronous function
-  /// and marks the span as errored if an exception occurs.
-  R traceSync<R>(String name, R Function() fn, {api.Context context});
-
-  /// Records a span of the given [name] for the given asynchronous function
+  /// Records a span of the given [name] for the given function
   /// and marks the span as errored if an exception occurs.
-  Future<R> traceAsync<R>(String name, Future<R> Function() fn,
+  FutureOr<R> trace<R>(String name, FutureOr<R> Function() fn,
       {api.Context context});
 }
diff --git a/lib/src/sdk/trace/tracer.dart b/lib/src/sdk/trace/tracer.dart
@@ -53,35 +53,22 @@ class Tracer implements api.Tracer {
         attributes: attributes);
   }
 
-  /// Records a span of the given [name] for the given synchronous function
-  /// and marks the span as errored if an exception occurs.
-  @override
-  R traceSync<R>(String name, R Function() fn, {api.Context context}) {
-    final operationContext = context ?? api.Context.current;
-    final span = startSpan(name, context: operationContext);
-
-    try {
-      return operationContext.withSpan(span).execute(fn);
-    } catch (e, s) {
-      span.recordException(e, stackTrace: s);
-      rethrow;
-    } finally {
-      span.end();
-    }
-  }
-
   /// Records a span of the given [name] for the given asynchronous function
   /// and marks the span as errored if an exception occurs.
   @override
-  Future<R> traceAsync<R>(String name, Future<R> Function() fn,
+  FutureOr<R> trace<R>(String name, FutureOr<R> Function() fn,
       {api.Context context}) async {
-    final operationContext = context ?? api.Context.current;
-    final span = startSpan(name, context: operationContext);
+    context ??= api.Context.current;
+    final span = startSpan(name, context: context);
 
     try {
-      // Operation must be awaited here to ensure the catch block intercepts
-      // errors thrown by [fn].
-      return await operationContext.withSpan(span).execute(fn);
+      var result = context.withSpan(span).execute(fn);
+      if (result is Future) {
+        // Operation must be awaited here to ensure the catch block intercepts
+        // errors thrown by [fn].
+        result = await result;
+      }
+      return result;
     } catch (e, s) {
       span.recordException(e, stackTrace: s);
       rethrow;

diff --git a/test/integration/sdk/tracer_test.dart b/test/integration/sdk/tracer_test.dart
@@ -42,22 +42,22 @@ void main() {
         allOf([isNotNull, isNot(equals(parentSpan.spanContext.spanId))]));
   });
 
-  test('traceSync execution', () {
+  test('trace synchronous execution', () {
     final tracer = sdk.Tracer([],
         sdk.Resource(api.Attributes.empty()),
         sdk.AlwaysOnSampler(),
         sdk.IdGenerator(),
         sdk.InstrumentationLibrary('name', 'version'));
     sdk.Span span;
 
-    tracer.traceSync('syncTrace', () {
+    tracer.trace('syncTrace', () {
       span = api.Context.current.span;
     });
 
     expect(span.endTime, lessThan(DateTime.now().microsecondsSinceEpoch));
   });
 
-  test('traceSync looped execution timing', () {
+  test('trace synchronous looped execution timing', () {
     final tracer = sdk.Tracer([],
         sdk.Resource(api.Attributes.empty()),
         sdk.AlwaysOnSampler(),
@@ -66,7 +66,7 @@ void main() {
     final spans = <sdk.Span>[];
 
     for (var i = 0; i < 5; i++) {
-      tracer.traceSync('syncTrace', () {
+      tracer.trace('syncTrace', () {
         spans.add(api.Context.current.span);
       });
     }
@@ -77,7 +77,7 @@ void main() {
     }
   });
 
-  test('traceSync execution with error', () {
+  test('trace synchronous execution with error', () {
     final tracer = sdk.Tracer([],
         sdk.Resource(api.Attributes.empty()),
         sdk.AlwaysOnSampler(),
@@ -86,7 +86,7 @@ void main() {
     sdk.Span span;
 
     expect(
-        () => tracer.traceSync('syncTrace', () {
+        () => tracer.trace('syncTrace', () {
               span = api.Context.current.span;
               throw Exception('Oh noes!');
             }),
@@ -98,22 +98,22 @@ void main() {
     expect(span.attributes.get('exception'), equals('Exception: Oh noes!'));
   });
 
-  test('traceAsync execution', () async {
+  test('trace asynchronous execution', () async {
     final tracer = sdk.Tracer([],
         sdk.Resource(api.Attributes.empty()),
         sdk.AlwaysOnSampler(),
         sdk.IdGenerator(),
         sdk.InstrumentationLibrary('name', 'version'));
     sdk.Span span;
 
-    await tracer.traceAsync('asyncTrace', () async {
+    await tracer.trace('asyncTrace', () async {
       span = api.Context.current.span;
     });
 
     expect(span.endTime, lessThan(DateTime.now().microsecondsSinceEpoch));
   });
 
-  test('traceAsync looped execution timing', () async {
+  test('trace asynchronous looped execution timing', () async {
     final tracer = sdk.Tracer([],
         sdk.Resource(api.Attributes.empty()),
         sdk.AlwaysOnSampler(),
@@ -122,7 +122,7 @@ void main() {
     final spans = <sdk.Span>[];
 
     for (var i = 0; i < 5; i++) {
-      await tracer.traceAsync('asyncTrace', () async {
+      await tracer.trace('asyncTrace', () async {
         spans.add(api.Context.current.span);
       });
     }
@@ -133,7 +133,7 @@ void main() {
     }
   });
 
-  test('traceAsync execution with thrown error', () async {
+  test('trace asynchronous execution with thrown error', () async {
     final tracer = sdk.Tracer([],
         sdk.Resource(api.Attributes.empty()),
         sdk.AlwaysOnSampler(),
@@ -142,7 +142,7 @@ void main() {
     sdk.Span span;
 
     try {
-      await tracer.traceAsync('asyncTrace', () async {
+      await tracer.trace('asyncTrace', () async {
         span = api.Context.current.span;
         throw Exception('Oh noes!');
       });
@@ -156,7 +156,7 @@ void main() {
     expect(span.attributes.get('exception'), equals('Exception: Oh noes!'));
   });
 
-  test('traceAsync execution completes with error', () async {
+  test('trace asynchronous execution completes with error', () async {
     final tracer = sdk.Tracer([],
         sdk.Resource(api.Attributes.empty()),
         sdk.AlwaysOnSampler(),
@@ -165,7 +165,7 @@ void main() {
     sdk.Span span;
 
     try {
-      await tracer.traceAsync('asyncTrace', () async {
+      await tracer.trace('asyncTrace', () async {
         span = api.Context.current.span;
         return Future.error(Exception('Oh noes!'));
       });