From b2d7551b840ad562d9590429e365dbeb7aa3c4dc Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Tue, 9 Oct 2018 13:52:36 -0600 Subject: [PATCH 01/14] AF-1540 consumer-specified timing --- .../panel/modules/data_load_async_module.dart | 29 ++-- lib/src/lifecycle_module.dart | 83 ++++++++++- lib/src/timing_specifiers.dart | 11 ++ lib/w_module.dart | 1 + test/lifecycle_module_test.dart | 139 +++++++++++++++++- 5 files changed, 240 insertions(+), 23 deletions(-) create mode 100644 lib/src/timing_specifiers.dart diff --git a/example/panel/modules/data_load_async_module.dart b/example/panel/modules/data_load_async_module.dart index 6231176d..cf67b23a 100644 --- a/example/panel/modules/data_load_async_module.dart +++ b/example/panel/modules/data_load_async_module.dart @@ -27,12 +27,14 @@ class DataLoadAsyncModule extends Module { DataLoadAsyncActions _actions; DataLoadAsyncComponents _components; - DataLoadAsyncStore _stores; + DataLoadAsyncEvents _events; + DataLoadAsyncStore _store; DataLoadAsyncModule() { _actions = new DataLoadAsyncActions(); - _stores = new DataLoadAsyncStore(_actions); - _components = new DataLoadAsyncComponents(_actions, _stores); + _events = new DataLoadAsyncEvents(); + _store = new DataLoadAsyncStore(_actions, _events); + _components = new DataLoadAsyncComponents(_actions, _store); } @override @@ -42,6 +44,8 @@ class DataLoadAsyncModule extends Module { @protected Future onLoad() { // trigger non-blocking async load of data + _events.didLoadData.first + .then((_) => specifyStartupTiming(StartupTimingType.firstUseful)); _actions.loadData(); return new Future.value(); } @@ -62,15 +66,20 @@ class DataLoadAsyncActions { final Action loadData = new Action(); } +DispatchKey _dispatchKey = new DispatchKey('DataLoadAsync'); + +class DataLoadAsyncEvents { + final Event didLoadData = new Event(_dispatchKey); +} + class DataLoadAsyncStore extends Store { DataLoadAsyncActions _actions; - List _data; - bool _isLoading; + DataLoadAsyncEvents _events; + List _data = []; + bool _isLoading = false; - DataLoadAsyncStore(this._actions) { - _isLoading = false; - _data = []; - triggerOnAction(_actions.loadData, _loadData); + DataLoadAsyncStore(this._actions, this._events) { + manageActionSubscription(_actions.loadData.listen(_loadData)); } /// Public data @@ -89,6 +98,8 @@ class DataLoadAsyncStore extends Store { // trigger on return of final data _data = ['Aaron', 'Dustin', 'Evan', 'Jay', 'Max', 'Trent']; _isLoading = false; + _events.didLoadData(null, _dispatchKey); + trigger(); } } diff --git a/lib/src/lifecycle_module.dart b/lib/src/lifecycle_module.dart index 94613d7a..55357d50 100644 --- a/lib/src/lifecycle_module.dart +++ b/lib/src/lifecycle_module.dart @@ -22,6 +22,7 @@ import 'package:opentracing/opentracing.dart'; import 'package:w_common/disposable.dart'; import 'package:w_module/src/simple_module.dart'; +import 'package:w_module/src/timing_specifiers.dart'; /// Possible states a [LifecycleModule] may occupy. enum LifecycleState { @@ -54,7 +55,7 @@ enum LifecycleState { /// unified lifecycle API. abstract class LifecycleModule extends SimpleModule with Disposable { static int _nextId = 0; - + // Used by tracing to tell apart multiple instances of the same module int _instanceId = _nextId++; List _childModules = []; @@ -64,7 +65,10 @@ abstract class LifecycleModule extends SimpleModule with Disposable { LifecycleState _state = LifecycleState.instantiated; Completer _transition; Span _activeSpan; - SpanContext _loadSpanContext; + + // Used by tracing to create a span if the consumer specifies when the module + // reaches its first useful state + DateTime _startLoadTime; // Lifecycle event StreamControllers StreamController _willLoadChildModuleController = @@ -153,6 +157,10 @@ abstract class LifecycleModule extends SimpleModule with Disposable { @protected Span get activeSpan => _activeSpan; + /// Set internally by this module for the load span so it can be used as a + /// `Reference` to other spans after the span is finished. + SpanContext _loadContext; + /// Set internally by the parent module if this module is called by [loadChildModule] SpanContext _parentContext; @@ -177,16 +185,74 @@ abstract class LifecycleModule extends SimpleModule with Disposable { references.add(new Reference.followsFrom(_parentContext)); } - return tracer.startSpan('$name.$operationName', references: references) - ..addTags({ - 'module.instanceId': _instanceId, - }); + return tracer.startSpan( + '$name.$operationName', + references: references, + tags: _defaultTags, + ); + } + + /// Creates a span with `globalTracer` from the start of [load] until now. + /// + /// This span is intended to represent the time it takes for the module to + /// finish asynchronously loading any necessary data and entering a state which + /// is ready for user interaction. + /// + /// Any [tags] or [references] specified will be added to this span. + @protected + void specifyFirstUsefulState({ + Map tags: const {}, + List references: const [], + }) => + specifyStartupTiming( + StartupTimingType.firstUseful, + tags: tags, + references: references, + ); + + /// Creates a span with `globalTracer` from the start of [load] until now. + /// + /// The [specifier] indicates the purpose of this span. + /// + /// Any [tags] or [references] specified will be added to this span. + @protected + void specifyStartupTiming( + StartupTimingType specifier, { + Map tags: const {}, + List references: const [], + }) { + // Load didn't start + if (_loadContext == null || _startLoadTime == null) { + throw new StateError( + 'Calling `specifyStartupTiming` before calling `load()`'); + } + + final tracer = globalTracer(); + if (tracer == null) { + return null; + } + + tracer + .startSpan( + '$name.${specifier.operationName}', + references: [tracer.followsFrom(_loadContext)]..addAll(references), + startTime: _startLoadTime, + tags: _defaultTags..addAll(tags), + ) + .finish(); + + _startLoadTime = null; } /// Name of the module for identification in exceptions and debug messages. // ignore: unnecessary_getters_setters String get name => _defaultName; + Map get _defaultTags => { + 'span.kind': 'client', + 'module.instanceId': _instanceId, + }; + /// Deprecated: the module name should be defined by overriding the getter in /// a subclass and it should not be mutable. @deprecated @@ -377,7 +443,8 @@ abstract class LifecycleModule extends SimpleModule with Disposable { } _activeSpan = _startTransitionSpan('load'); - _loadSpanContext = _activeSpan?.context; + _loadContext = _activeSpan?.context; + _startLoadTime = _activeSpan?.startTime; _state = LifecycleState.loading; @@ -455,7 +522,7 @@ abstract class LifecycleModule extends SimpleModule with Disposable { try { _childModules.add(childModule); - childModule._parentContext = _loadSpanContext; + childModule._parentContext = _loadContext; await childModule.load(); await onDidLoadChildModule(childModule); diff --git a/lib/src/timing_specifiers.dart b/lib/src/timing_specifiers.dart new file mode 100644 index 00000000..f1e63203 --- /dev/null +++ b/lib/src/timing_specifiers.dart @@ -0,0 +1,11 @@ +/// The type of 'startup timing metric' to be used by `specifyStartupTiming` +class StartupTimingType { + /// The `operationName` to be used for spans created using this [StartupTimingType]. + final String operationName; + + const StartupTimingType._(this.operationName); + + /// Specifies that the module finished loading necessary data and is ready for user interaction. + static const StartupTimingType firstUseful = + const StartupTimingType._('entered_first_useful_state'); +} diff --git a/lib/w_module.dart b/lib/w_module.dart index 374c2e38..2467cdd5 100644 --- a/lib/w_module.dart +++ b/lib/w_module.dart @@ -26,3 +26,4 @@ export 'package:w_module/src/events_collection.dart'; export 'package:w_module/src/lifecycle_module.dart' hide LifecycleState; export 'package:w_module/src/module.dart'; export 'package:w_module/src/simple_module.dart'; +export 'package:w_module/src/timing_specifiers.dart'; diff --git a/test/lifecycle_module_test.dart b/test/lifecycle_module_test.dart index b613a133..b59dbf69 100644 --- a/test/lifecycle_module_test.dart +++ b/test/lifecycle_module_test.dart @@ -22,6 +22,7 @@ import 'package:opentracing/opentracing.dart'; import 'package:test/test.dart'; import 'package:w_module/src/lifecycle_module.dart'; +import 'package:w_module/src/timing_specifiers.dart'; import 'test_tracer.dart'; import 'utils.dart'; @@ -110,6 +111,23 @@ class TestLifecycleModule extends LifecycleModule { Future loadChildModule(LifecycleModule newModule) => super.loadChildModule(newModule); + @override + void specifyFirstUsefulState({ + Map tags: const {}, + List references: const [], + }) => + super.specifyFirstUsefulState(tags: tags, references: references); + + // Overriding without re-applying the @protected annotation allows us to call + // specifyStartupTiming in our tests below. + @override + void specifyStartupTiming( + StartupTimingType specifier, { + Map tags: const {}, + List references: const [], + }) => + super.specifyStartupTiming(specifier, tags: tags, references: references); + @override @protected Future onWillLoadChildModule(LifecycleModule module) async { @@ -346,6 +364,15 @@ TestTracer getTestTracer() { } void runTests(bool runSpanTests) { + test('Calling `specifyStartupTiming` without calling `load()` throws', () { + final module = new TestLifecycleModule(); + + expect( + () => module.specifyStartupTiming(StartupTimingType.firstUseful), + throwsStateError, + ); + }); + group('without children', () { TestLifecycleModule module; List subs = []; @@ -409,6 +436,60 @@ void runTests(bool runSpanTests) { await module.load(); }); + group('should record user specified timing', () { + DateTime startTime; + + setUp(() async { + final Completer startTimeCompleter = new Completer(); + + subs.add(getTestTracer() + .onSpanFinish + .where( + (span) => span.operationName == 'TestLifecycleModule.load') + .listen(expectAsync1((span) { + startTimeCompleter.complete(span.startTime); + }))); + + await module.load(); + + startTime = await startTimeCompleter.future; + }); + + tearDown(() { + startTime = null; + }); + + [ + StartupTimingType.firstUseful, + ].forEach((specifier) { + test('should specify timing for ${specifier.operationName}', + () async { + subs.add(getTestTracer() + .onSpanFinish + .where((span) => + span.operationName == + 'TestLifecycleModule.${specifier.operationName}') + .listen(expectAsync1((span) { + expect(span.startTime, startTime); + }))); + module.specifyStartupTiming(specifier); + }); + }); + + test('shorthand for firstUseful timing', () { + subs.add(getTestTracer() + .onSpanFinish + .where((span) => + span.operationName == + 'TestLifecycleModule.${StartupTimingType.firstUseful.operationName}') + .listen(expectAsync1((span) { + expect(span.startTime, startTime); + }))); + + module.specifyFirstUsefulState(); + }); + }); + test('activeSpan should be null when load is finished', () async { await module.load(); expect(module.activeSpan, isNull); @@ -1050,6 +1131,21 @@ void runTests(bool runSpanTests) { await module.resume(); }); + if (runSpanTests) { + test('should record a span', () async { + await gotoState(module, LifecycleState.suspended); + + subs.add(getTestTracer() + .onSpanFinish + .where( + (span) => span.operationName == 'TestLifecycleModule.resume') + .listen(expectAsync1((span) { + expect(span.tags['custom.resume.tag'], 'somevalue'); + }))); + + await module.resume(); + }); + } test('an error in suspend bubbles up during resume', () async { await gotoState(module, LifecycleState.loaded); @@ -1777,6 +1873,34 @@ void runTests(bool runSpanTests) { equals(['willSuspend', 'onSuspend', 'didSuspend'])); }); + if (runSpanTests) { + test('child module suspends should record spans', () async { + Completer childSpanCompleter = new Completer(); + + subs.add(getTestTracer() + .onSpanFinish + .where((span) => span.operationName == 'child.suspend') + .listen(expectAsync1((span) { + childSpanCompleter.complete(span); + }))); + + parentModule.eventList.clear(); + childModule.eventList.clear(); + await parentModule.suspend(); + expect(parentModule.eventList, + equals(['willSuspend', 'onSuspend', 'didSuspend'])); + expect(childModule.eventList, + equals(['willSuspend', 'onSuspend', 'didSuspend'])); + + final span = await childSpanCompleter.future; + await new Future(() {}); // wait for parent to finish suspending + + expect(parentSuspendContext?.spanId, isNotNull); + expect(span.parentContext.spanId, parentSuspendContext.spanId); + expect(span.tags['custom.suspend.tag'], 'somevalue'); + }); + } + test('an error in suspend bubbles up during resume', () async { assert(parentModule.isLoaded); @@ -2364,7 +2488,7 @@ void runTests(bool runSpanTests) { }); if (runSpanTests) { - test('should record a span for child with no parent', () async { + test('should record a span for child when parent has no name', () async { subs.add(getTestTracer() .onSpanFinish .where((span) => span.operationName == 'child.load') @@ -2376,7 +2500,8 @@ void runTests(bool runSpanTests) { await parentModule.loadChildModule(childModule); }); - test('should record a span with `error` tag and no parent', () async { + test('should record a span with `error` tag when parent has no name', + () async { childModule.onLoadError = testError; subs.add(getTestTracer() @@ -2391,7 +2516,8 @@ void runTests(bool runSpanTests) { throwsA(same(childModule.onLoadError))); }); - test('child module suspend should record spans with no parent', () async { + test('child module suspend should record spans when parent has no name', + () async { await parentModule.loadChildModule(childModule); subs.add(getTestTracer() .onSpanFinish @@ -2405,7 +2531,7 @@ void runTests(bool runSpanTests) { }); test( - 'child module suspend throws should record a span with `error` tag and no parent', + 'child module suspend throws should record a span with `error` tag and parent has no name', () async { await parentModule.loadChildModule(childModule); childModule.onSuspendError = testError; @@ -2422,7 +2548,8 @@ void runTests(bool runSpanTests) { parentModule.suspend(), throwsA(same(childModule.onSuspendError))); }); - test('child module resume should record a span with no parent', () async { + test('child module resume should record a span when parent has no name', + () async { await parentModule.loadChildModule(childModule); subs.add(getTestTracer() @@ -2438,7 +2565,7 @@ void runTests(bool runSpanTests) { }); test( - 'child module resume should record a span with `error` tag and no parent', + 'child module resume should record a span with `error` tag and parent has no name', () async { await parentModule.loadChildModule(childModule); childModule.onResumeError = testError; From 89b17235e2983da1d21c5c70f0c0f0a4070e3d34 Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Wed, 10 Oct 2018 11:15:05 -0600 Subject: [PATCH 02/14] AF-1540 test custom tags and refs with specified timing --- .../panel/modules/data_load_async_module.dart | 4 +- pubspec.yaml | 2 +- test/lifecycle_module_test.dart | 60 ++++++++++++------- test/test_tracer.dart | 2 +- 4 files changed, 44 insertions(+), 24 deletions(-) diff --git a/example/panel/modules/data_load_async_module.dart b/example/panel/modules/data_load_async_module.dart index cf67b23a..8b7570d8 100644 --- a/example/panel/modules/data_load_async_module.dart +++ b/example/panel/modules/data_load_async_module.dart @@ -44,8 +44,8 @@ class DataLoadAsyncModule extends Module { @protected Future onLoad() { // trigger non-blocking async load of data - _events.didLoadData.first - .then((_) => specifyStartupTiming(StartupTimingType.firstUseful)); + listenToStream(_events.didLoadData.take(1), + (_) => specifyStartupTiming(StartupTimingType.firstUseful)); _actions.loadData(); return new Future.value(); } diff --git a/pubspec.yaml b/pubspec.yaml index 4eb55695..b896cdde 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -13,7 +13,7 @@ homepage: https://github.com/Workiva/w_module dependencies: logging: ^0.11.0 meta: ^1.0.0 - opentracing: ^0.2.1 + opentracing: ^0.2.3 platform_detect: ^1.1.0 w_common: ^1.9.0 diff --git a/test/lifecycle_module_test.dart b/test/lifecycle_module_test.dart index b59dbf69..026f6d39 100644 --- a/test/lifecycle_module_test.dart +++ b/test/lifecycle_module_test.dart @@ -438,6 +438,7 @@ void runTests(bool runSpanTests) { group('should record user specified timing', () { DateTime startTime; + Span parentSpan; setUp(() async { final Completer startTimeCompleter = new Completer(); @@ -453,40 +454,59 @@ void runTests(bool runSpanTests) { await module.load(); startTime = await startTimeCompleter.future; + + parentSpan = getTestTracer().startSpan('custom span')..finish(); }); tearDown(() { startTime = null; }); - [ - StartupTimingType.firstUseful, - ].forEach((specifier) { - test('should specify timing for ${specifier.operationName}', - () async { - subs.add(getTestTracer() - .onSpanFinish - .where((span) => - span.operationName == - 'TestLifecycleModule.${specifier.operationName}') - .listen(expectAsync1((span) { - expect(span.startTime, startTime); - }))); - module.specifyStartupTiming(specifier); - }); - }); - - test('shorthand for firstUseful timing', () { + void specifyTimingTest( + StartupTimingType specifier, + void specifyDelegate( + {Map tags, List references}), + ) { subs.add(getTestTracer() .onSpanFinish .where((span) => span.operationName == - 'TestLifecycleModule.${StartupTimingType.firstUseful.operationName}') + 'TestLifecycleModule.${specifier.operationName}') .listen(expectAsync1((span) { expect(span.startTime, startTime); + expect(span.tags, containsPair('custom.tag', 'custom value')); + expect(span.references.length, 2); + expect( + span.references, + anyElement((Reference ref) => + ref.referencedContext == parentSpan.context)); }))); - module.specifyFirstUsefulState(); + specifyDelegate( + tags: {'custom.tag': 'custom value'}, + references: [getTestTracer().followsFrom(parentSpan.context)], + ); + } + + ; + + [ + StartupTimingType.firstUseful, + ].forEach((specifier) { + test('specifyStartupTiming for ${specifier.operationName}', () { + specifyTimingTest( + specifier, + ({tags, references}) => module.specifyStartupTiming( + specifier, + tags: tags, + references: references, + )); + }); + }); + + test('shorthand for firstUseful timing', () { + specifyTimingTest( + StartupTimingType.firstUseful, module.specifyFirstUsefulState); }); }); diff --git a/test/test_tracer.dart b/test/test_tracer.dart index ec1f5ef7..0df40080 100644 --- a/test/test_tracer.dart +++ b/test/test_tracer.dart @@ -35,7 +35,7 @@ class TestSpan implements Span { }) : this.startTime = startTime ?? new DateTime.now(), this.tags = tags ?? {}, - this.references = references ?? {} { + this.references = references ?? [] { if (childOf != null) { references.add(new Reference.childOf(childOf)); } From 905a80ccb779da56b1eb91618576d83f0f852a8a Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Wed, 10 Oct 2018 11:30:47 -0600 Subject: [PATCH 03/14] update smithy to 1.24.3 --- Dockerfile | 2 +- smithy.yaml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index d7e2d59b..1e1c317a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM drydock-prod.workiva.net/workiva/smithy-runner-generator:203768 as build +FROM drydock-prod.workiva.net/workiva/smithy-runner-generator:269160 as build # Build Environment Vars ARG BUILD_ID diff --git a/smithy.yaml b/smithy.yaml index 9fac396e..ad945f35 100644 --- a/smithy.yaml +++ b/smithy.yaml @@ -1,8 +1,8 @@ project: dart language: dart -# dart 1.24.2 -runner_image: drydock-prod.workiva.net/workiva/smithy-runner-generator:203768 +# dart 1.24.3 +runner_image: drydock-prod.workiva.net/workiva/smithy-runner-generator:269160 script: - pub get --packages-dir From aab0caeaf537d4e1095dcbb7537299907d5a938f Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Wed, 10 Oct 2018 11:32:30 -0600 Subject: [PATCH 04/14] leftover semicolon --- test/lifecycle_module_test.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/lifecycle_module_test.dart b/test/lifecycle_module_test.dart index 026f6d39..2a3832c3 100644 --- a/test/lifecycle_module_test.dart +++ b/test/lifecycle_module_test.dart @@ -488,8 +488,6 @@ void runTests(bool runSpanTests) { ); } - ; - [ StartupTimingType.firstUseful, ].forEach((specifier) { From 3d3b87b3f6b71e4426c502090cfc9c511f7be8ac Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Wed, 10 Oct 2018 11:40:02 -0600 Subject: [PATCH 05/14] use google dart image --- Dockerfile | 2 +- smithy.yaml | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index 1e1c317a..02f26b67 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM drydock-prod.workiva.net/workiva/smithy-runner-generator:269160 as build +FROM google/dart:1.24.3 as build # Build Environment Vars ARG BUILD_ID diff --git a/smithy.yaml b/smithy.yaml index ad945f35..a2277458 100644 --- a/smithy.yaml +++ b/smithy.yaml @@ -1,8 +1,7 @@ project: dart language: dart -# dart 1.24.3 -runner_image: drydock-prod.workiva.net/workiva/smithy-runner-generator:269160 +runner_image: google/dart:1.24.3 script: - pub get --packages-dir From 5fad098e54d58ec85c01732b629816b3440151a2 Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Wed, 10 Oct 2018 11:45:04 -0600 Subject: [PATCH 06/14] more image stuff --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 02f26b67..84437b14 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,7 +15,7 @@ WORKDIR /build/ ADD . /build/ RUN echo "Starting the script sections" && \ pub get --packages-dir && \ - xvfb-run -s '-screen 0 1024x768x24' pub run dart_dev test --pub-serve --web-compiler=dartdevc -p chrome -p vm && \ + pub run dart_dev test --pub-serve --web-compiler=dartdevc -p chrome -p vm && \ echo "Script sections completed" ARG BUILD_ARTIFACTS_BUILD=/build/pubspec.lock FROM scratch From 51cea1fbbd068ee729012094e1124742cc36f2eb Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Wed, 10 Oct 2018 12:17:49 -0600 Subject: [PATCH 07/14] install chrome in Dockerfile --- Dockerfile | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 84437b14..545dd8a5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,22 @@ FROM google/dart:1.24.3 as build +RUN apt-get update -qq +RUN apt-get update && apt-get install -y \ + build-essential \ + curl \ + git \ + make \ + wget \ + && rm -rf /var/lib/apt/lists/* + +# install chrome +RUN wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add - && \ + echo 'deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main' | tee /etc/apt/sources.list.d/google-chrome.list && \ + apt-get -qq update && apt-get install -y google-chrome-stable && \ + mv /usr/bin/google-chrome-stable /usr/bin/google-chrome && \ + sed -i --follow-symlinks -e 's/\"\$HERE\/chrome\"/\"\$HERE\/chrome\" --no-sandbox/g' /usr/bin/google-chrome && \ + google-chrome --version + # Build Environment Vars ARG BUILD_ID ARG BUILD_NUMBER @@ -15,7 +32,7 @@ WORKDIR /build/ ADD . /build/ RUN echo "Starting the script sections" && \ pub get --packages-dir && \ - pub run dart_dev test --pub-serve --web-compiler=dartdevc -p chrome -p vm && \ + xvfb-run -s '-screen 0 1024x768x24' pub run dart_dev test --pub-serve --web-compiler=dartdevc -p chrome -p vm && \ echo "Script sections completed" ARG BUILD_ARTIFACTS_BUILD=/build/pubspec.lock FROM scratch From fc57bd05ea4fd088da9f731c63c5e504953b23f5 Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Wed, 10 Oct 2018 12:24:53 -0600 Subject: [PATCH 08/14] install xvfb --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index 545dd8a5..cf6991a0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,6 +6,7 @@ RUN apt-get update && apt-get install -y \ curl \ git \ make \ + xvfb \ wget \ && rm -rf /var/lib/apt/lists/* From 52793c36ececfe16d290c92decda995e5e24eb09 Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Wed, 10 Oct 2018 13:15:26 -0600 Subject: [PATCH 09/14] anyElement is borked in chrome test --- test/lifecycle_module_test.dart | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/lifecycle_module_test.dart b/test/lifecycle_module_test.dart index 2a3832c3..946355dd 100644 --- a/test/lifecycle_module_test.dart +++ b/test/lifecycle_module_test.dart @@ -476,10 +476,8 @@ void runTests(bool runSpanTests) { expect(span.startTime, startTime); expect(span.tags, containsPair('custom.tag', 'custom value')); expect(span.references.length, 2); - expect( - span.references, - anyElement((Reference ref) => - ref.referencedContext == parentSpan.context)); + expect(span.references.map((ref) => ref.referencedContext), + contains(parentSpan.context)); }))); specifyDelegate( From 917660ce59e051b7125817c2204b9b09bdd6e9da Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Wed, 10 Oct 2018 15:10:27 -0600 Subject: [PATCH 10/14] remove smithy --- smithy.yaml | 13 ------------- 1 file changed, 13 deletions(-) delete mode 100644 smithy.yaml diff --git a/smithy.yaml b/smithy.yaml deleted file mode 100644 index a2277458..00000000 --- a/smithy.yaml +++ /dev/null @@ -1,13 +0,0 @@ -project: dart -language: dart - -runner_image: google/dart:1.24.3 - -script: - - pub get --packages-dir - - xvfb-run -s '-screen 0 1024x768x24' pub run dart_dev test --pub-serve --web-compiler=dartdevc -p chrome -p vm - -artifacts: - build: - - ./pubspec.lock - From 18919d150ec391a66ccedbcca6fbadced5b4101a Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Thu, 11 Oct 2018 11:05:15 -0600 Subject: [PATCH 11/14] AF-2674 documentation for w_module tracing --- docs.yml | 5 ++- documentation/tracing.md | 73 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 documentation/tracing.md diff --git a/docs.yml b/docs.yml index c982e6ab..ea25afd3 100644 --- a/docs.yml +++ b/docs.yml @@ -1,3 +1,6 @@ title: w_module base: github:Workiva/w_module/ -src: README.md \ No newline at end of file +src: README.md +topics: + - title: Tracing with w_module + src: docs/tracing.md diff --git a/documentation/tracing.md b/documentation/tracing.md new file mode 100644 index 00000000..1f9fc54b --- /dev/null +++ b/documentation/tracing.md @@ -0,0 +1,73 @@ +# Tracing with w_module + +This library supports [OpenTracing][opentracingio] using [opentracing_dart][opentracingdart]. Your application will need to provide a `Tracer` and initialize it with `initGlobalTracer` to opt in to this feature. + + +To get the traces provided by this library, your module must provide a definition for the `name` getter. This can simply be the name of the class. For example: + +```dart +class SomeModule extends Module { + @override + final String name = 'SomeModule'; + + // ... the of the module's implementation +} +``` + +Spans will be in the form of: + +``` +$name.$operationName +``` + +## Types Of Provided Traces + +### Tracing Lifecycle Methods + +We automically trace each of its lifecycle methods: + +- Load +- Unload +- Suspend +- Resume + +In addition, any spans created by child modules (loaded with `loadChildModule`) will have a `followsFrom` reference to the parent's span of the respective method to complete the story of the trace. + +If you wish to create other `childOf` or `followsFrom` spans on your module's lifecycle spans, you can simply request the `activeSpan`: + +```dart + @override + Future onLoad() { + // ... some loading logic + + final span = globalTracer().startSpan( + operationName, + childOf: activeSpan.context, // see this line + ); + + // ... more loading logic + span.finish() + } +``` + +Note that `activeSpan` will be null at times when the module is not in the middle of a lifecycle transition. + +### Additional Load Time Granularity + +Sometimes, lifecycle methods such as `load` will complete before the module is semantically "loaded". For example, you may begin asynchronously fetching data for your module and then return from `onLoad` to keep from blocking the main thread. + +In cases like these, use `specifyStartupTiming`: + +``` + Future onLoad() { + // ... kick off async loadData logic + + listenToStream(_events.didLoadData.take(1), + (_) => specifyStartupTiming(StartupTimingType.firstUseful)); + } +``` + +This will create a span starting from the same time as `load()` and ending at the moment the method was called. This library will handle the `operationName` and the `followsFrom` reference to the module's `load` span, but tags and references can be passed into this method just like with any other span in optional parameters. + +[opentracingio]: https://opentracing.io/ +[opentracingdart]: https://github.com/Workiva/opentracing_dart/ \ No newline at end of file From 2240934c8690a93edf117ce51039e342415b8ec4 Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Fri, 12 Oct 2018 15:12:44 -0600 Subject: [PATCH 12/14] AF-1540 address nits --- documentation/tracing.md | 2 +- example/panel/modules/data_load_async_module.dart | 2 ++ lib/src/lifecycle_module.dart | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/documentation/tracing.md b/documentation/tracing.md index 1f9fc54b..1ba41e5f 100644 --- a/documentation/tracing.md +++ b/documentation/tracing.md @@ -58,7 +58,7 @@ Sometimes, lifecycle methods such as `load` will complete before the module is s In cases like these, use `specifyStartupTiming`: -``` +```dart Future onLoad() { // ... kick off async loadData logic diff --git a/example/panel/modules/data_load_async_module.dart b/example/panel/modules/data_load_async_module.dart index 8b7570d8..6dd01a5e 100644 --- a/example/panel/modules/data_load_async_module.dart +++ b/example/panel/modules/data_load_async_module.dart @@ -35,6 +35,8 @@ class DataLoadAsyncModule extends Module { _events = new DataLoadAsyncEvents(); _store = new DataLoadAsyncStore(_actions, _events); _components = new DataLoadAsyncComponents(_actions, _store); + + [_actions, _events, _store].forEach(manageDisposable); } @override diff --git a/lib/src/lifecycle_module.dart b/lib/src/lifecycle_module.dart index 55357d50..22b43952 100644 --- a/lib/src/lifecycle_module.dart +++ b/lib/src/lifecycle_module.dart @@ -250,7 +250,7 @@ abstract class LifecycleModule extends SimpleModule with Disposable { Map get _defaultTags => { 'span.kind': 'client', - 'module.instanceId': _instanceId, + 'module.instance_id': _instanceId, }; /// Deprecated: the module name should be defined by overriding the getter in From b40c1f4bbfcba081cd3b26413953cc08503a272d Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Fri, 12 Oct 2018 15:25:48 -0600 Subject: [PATCH 13/14] actions arent disposable --- example/panel/modules/data_load_async_module.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/panel/modules/data_load_async_module.dart b/example/panel/modules/data_load_async_module.dart index 6dd01a5e..ca8542ec 100644 --- a/example/panel/modules/data_load_async_module.dart +++ b/example/panel/modules/data_load_async_module.dart @@ -36,7 +36,7 @@ class DataLoadAsyncModule extends Module { _store = new DataLoadAsyncStore(_actions, _events); _components = new DataLoadAsyncComponents(_actions, _store); - [_actions, _events, _store].forEach(manageDisposable); + [_events, _store].forEach(manageDisposable); } @override From 4d4024234c6924b98f559831193f93c997fe2f36 Mon Sep 17 00:00:00 2001 From: Todd Beckman Date: Fri, 12 Oct 2018 15:37:16 -0600 Subject: [PATCH 14/14] better style and fix Events not being Disposable --- example/panel/modules/data_load_async_module.dart | 8 ++++++-- example/panel/modules/sample_tracer.dart | 3 +-- lib/src/lifecycle_module.dart | 4 +--- pubspec.yaml | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/example/panel/modules/data_load_async_module.dart b/example/panel/modules/data_load_async_module.dart index ca8542ec..b11de0c1 100644 --- a/example/panel/modules/data_load_async_module.dart +++ b/example/panel/modules/data_load_async_module.dart @@ -18,6 +18,7 @@ import 'dart:async'; import 'package:meta/meta.dart' show protected; import 'package:react/react.dart' as react; +import 'package:w_common/disposable.dart'; import 'package:w_flux/w_flux.dart'; import 'package:w_module/w_module.dart'; @@ -36,7 +37,7 @@ class DataLoadAsyncModule extends Module { _store = new DataLoadAsyncStore(_actions, _events); _components = new DataLoadAsyncComponents(_actions, _store); - [_events, _store].forEach(manageDisposable); + [_events, _store].forEach(manageDisposable); } @override @@ -70,8 +71,11 @@ class DataLoadAsyncActions { DispatchKey _dispatchKey = new DispatchKey('DataLoadAsync'); -class DataLoadAsyncEvents { +class DataLoadAsyncEvents extends EventsCollection { final Event didLoadData = new Event(_dispatchKey); + DataLoadAsyncEvents() : super(_dispatchKey) { + manageEvent(new Event(_dispatchKey)); + } } class DataLoadAsyncStore extends Store { diff --git a/example/panel/modules/sample_tracer.dart b/example/panel/modules/sample_tracer.dart index c636944e..3032a5f1 100644 --- a/example/panel/modules/sample_tracer.dart +++ b/example/panel/modules/sample_tracer.dart @@ -130,8 +130,7 @@ class SampleTracer implements AbstractTracer { references: references, startTime: startTime, tags: tags, - ) - ..whenFinished.then((span) { + )..whenFinished.then((span) { print(span.toString()); }); } diff --git a/lib/src/lifecycle_module.dart b/lib/src/lifecycle_module.dart index 22b43952..26cca641 100644 --- a/lib/src/lifecycle_module.dart +++ b/lib/src/lifecycle_module.dart @@ -120,9 +120,7 @@ abstract class LifecycleModule extends SimpleModule with Disposable { _didUnloadController, ].forEach(manageStreamController); - < - String, - Stream>{ + { 'willLoad': willLoad, 'didLoad': didLoad, 'willLoadChildModule': willLoadChildModule, diff --git a/pubspec.yaml b/pubspec.yaml index b896cdde..7b9b6415 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -21,8 +21,8 @@ dev_dependencies: browser: ^0.10.0+2 coverage: ^0.7.3 dart_dev: ^1.8.0 - dart_style: ^0.2.16 - dartdoc: ^0.9.0 + dart_style: ^1.0.7 + dartdoc: ">=0.13.0 <0.16.0" mockito: ^1.0.1 react: ^3.0.0 test: ^0.12.0