AF-3505 : ActiveSpan #34

dustinlessard-wf · 2018-12-15T17:36:52Z

Description

For platforms not propagating the call-context, it's inconvenient to pass the active Span from function to function manually, so OpenTracing should provide, for those platforms, that Tracer contains a Scope Manager that grants access to the active Span through a container, called Scope (using some call-context storage, such as thread-local or coroutine-local)

Driving use case: We want to trace what happens from the time a pre-authenticated user navigates to Wdesk, until they have arrived at home and can see the file/folder list populated.

To permit this, we need to support the concept of ScopeManager and ActiveSpan in opentracing_dart.

Changes

Added AbstractScope
Added AbstractScopeManager
Added startTime setter to AbstractSpan
Added ScopeManager getter to AbstractTracer
Added activeSpan getter to AbstractTracer
Added NoOpScope
Added NoOpScopeManager
Switched to using a single instance for NoOpSpanContext in NoOpSpan
Switched to using a single instance for a NoOpTracer's ScopeManager
Added various tests.
Some opportunistic refactors around lints.

Testing/QA

CI passes

Code Review

@Workiva/app-frameworks

aviary2-wf · 2018-12-15T17:37:12Z

Security Insights

No security relevant content was detected by automated scans.

Action Items

Review PR for security impact; comment "security review required" if needed or unsure
Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

toddbeckman-wf · 2018-12-20T15:21:10Z

example/default/example.dart

@@ -50,15 +50,16 @@ void runFailureCase() {

  HttpRequest.getString('http://httpstat.us/500').then((String result) {
    span.log('data_received', payload: result);
-  }).catchError((error) {
+  }).catchError((dynamic error) {


Dart recommends not specifying types in these anonymous functions. Is there a reason we are declaring this as dynamic? If so, would it be better to inline a type cast to be more explicit?

I was getting an analyzer warning to 'Specify type annotations'.

toddbeckman-wf · 2018-12-20T15:22:14Z

example/default/json_serializable_span.dart

 class JsonSerializableSpan {
+  // ignore: public_member_api_docs


BTW you can ignore this lint for the whole file (https://www.dartlang.org/guides/language/analysis-options#ignoring-specific-analysis-rules)

// ignore_for_file: public_member_api_docs

toddbeckman-wf · 2018-12-20T15:23:10Z

lib/noop_tracer.dart

 export 'src/noop_span.dart';
 export 'src/noop_span_context.dart';
 export 'src/noop_tracer.dart';
+export 'src/reference.dart';
+export 'src/reference.dart';
+export 'src/span_context.dart';


This and the line above it are repeated export statements.

toddbeckman-wf · 2018-12-20T15:24:18Z

lib/noop_tracer.dart

 export 'src/noop_span.dart';
 export 'src/noop_span_context.dart';
 export 'src/noop_tracer.dart';
+export 'src/reference.dart';
+export 'src/reference.dart';
+export 'src/span_context.dart';


Also, do we need to export this stuff through Noop Tracer? It makes more sense to me that they would import the normal library entrypoint to get access to these things, and use this entrypoint for just the Noop-related exports.

toddbeckman-wf · 2018-12-20T15:28:54Z

lib/src/noop_scope.dart

+import 'package:opentracing/opentracing.dart';
+
+/// The No-op implementation of [Scope] in which all operations are no-op
+class NoOpScope implements Scope {


Could we use Noop rather than NoOp for consistency?

danielharasymiw-wf · 2018-12-20T15:50:49Z

Maybe I missed it, or it's not needed. Do we also need to add a method to set the active span?
As well perhaps this could be done in future work, but I think opentracing java has an ignoreActiveSpan method?

dustinlessard-wf · 2018-12-21T12:06:01Z

@danielharasymiw-wf said:

Maybe I missed it, or it's not needed. Do we also need to add a method to set the active span?
As well perhaps this could be done in future work, but I think opentracing java has an ignoreActiveSpan method?

We can set the active span by calling

Tracer.scope.activate(theActiveSpan);

As well perhaps this could be done in future work, but I think opentracing java has an ignoreActiveSpan method?

I did indeed see that method here:
https://github.com/opentracing/opentracing-java#starting-a-new-span
From what I've seen, this method is related to the builders we see in the Java port, which don't exist in this Dart library. My preference is to leave that out for the time being.

Thoughts? @danielharasymiw-wf

sebastianmalysa-wf · 2018-12-27T19:05:17Z

test/unit/noop_scope_test.dart

+import 'package:opentracing/noop_tracer.dart';
+
+void main() {
+  group('NoOpScope: verify', () {


nit: s/NoOpScope/NoopScope here and in the test descriptions below

sebastianmalysa-wf · 2018-12-27T19:06:10Z

test/unit/noop_scope_manager_test.dart

+import 'package:opentracing/noop_tracer.dart';
+
+void main() {
+  group('NoOpScopeManager: verify', () {


nit: s/NoOpScope/NoopScope here and in the test descriptions below

sebastianmalysa-wf · 2018-12-27T19:07:27Z

test/unit/noop_tracer_test.dart

@@ -39,4 +39,10 @@ void main() {
    expect(noopSpan.context.sampled, expectedContext.sampled);
    expect(noopSpan.context.baggage, expectedContext.baggage);
  });
+
+  test('Verify startSpan returns NoOpSpan from NoOpScopeManager', () {


nit: s/NoOp/Noop in NoOpSpan, NoOpScopeManager and noOpTracer below

sebastianmalysa-wf

just a few small nits otherwise looks good to me

greglittlefield-wf

Looks good besides a few small things (and sorry for all the doc nits).

Have you spiked out a reference implementation of this that consumes these classes? I'm a little leery of committing to a public interface that doesn't have any real-world concrete implementations.

Also, a bigger question: are we trying to go off of this ScopeManager proposal, or just the Java impl minus the builder stuff?

greglittlefield-wf · 2018-12-28T16:52:46Z

lib/src/abstract_scope.dart

+/// A [Scope] formalizes the activation and deactivation of a [Span], usually
+/// from a CPU standpoint.
+///
+/// Many times a [Span] will be extant ([Span].finish() has not been called)


#nit Fix dartdoc reference

Suggested change

/// Many times a [Span] will be extant ([Span].finish() has not been called)

/// Many times a [Span] will be extant ([Span.finish] has not been called)

greglittlefield-wf · 2018-12-28T16:57:23Z

lib/src/abstract_scope.dart

@@ -0,0 +1,21 @@
+import 'package:opentracing/opentracing.dart';
+
+/// A [Scope] formalizes the activation and deactivation of a [Span], usually


#nit the Dart style guide discourages redundant references to the surrounding context

Suggested change

/// A [Scope] formalizes the activation and deactivation of a [Span], usually

/// A Scope formalizes the activation and deactivation of a [Span], usually

greglittlefield-wf · 2018-12-28T16:57:56Z

lib/src/abstract_scope.dart

+/// Many times a [Span] will be extant ([Span].finish() has not been called)
+/// despite being in a non-runnable state from a CPU/scheduler standpoint. For
+/// instance, a [Span] representing the client side of an RPC will be unfinished
+/// but blocked on IO while the RPC is still outstanding. A [Scope] defines when


#nit

Suggested change

/// but blocked on IO while the RPC is still outstanding. A [Scope] defines when

/// but blocked on IO while the RPC is still outstanding. A Scope defines when

greglittlefield-wf · 2018-12-28T16:58:15Z

lib/src/abstract_scope.dart

+  /// Mark the end of the active period for the current thread and [Scope],
+  /// updating the [ScopeManager].active() in the process.
+  ///
+  /// NOTE: Calling 'close' more than once on a single {@link Scope} instance


#nit

Suggested change

/// NOTE: Calling 'close' more than once on a single {@link Scope} instance

/// NOTE: Calling 'close' more than once on a single Scope instance

greglittlefield-wf · 2018-12-28T16:58:27Z

lib/src/abstract_scope.dart

+/// but blocked on IO while the RPC is still outstanding. A [Scope] defines when
+/// a given [Span] is scheduled and on the path.
+abstract class Scope {
+  /// Mark the end of the active period for the current thread and [Scope],


#nit

Suggested change

/// Mark the end of the active period for the current thread and [Scope],

/// Mark the end of the active period for the current thread and Scope,

greglittlefield-wf · 2018-12-28T17:06:12Z

lib/src/abstract_scope.dart

+/// a given [Span] is scheduled and on the path.
+abstract class Scope {
+  /// Mark the end of the active period for the current thread and [Scope],
+  /// updating the [ScopeManager].active() in the process.


#nit

Suggested change

/// updating the [ScopeManager].active() in the process.

/// updating the [ScopeManager.active] in the process.

greglittlefield-wf · 2018-12-28T17:07:24Z

lib/src/abstract_scope.dart

+  /// Mark the end of the active period for the current thread and [Scope],
+  /// updating the [ScopeManager].active() in the process.
+  ///
+  /// NOTE: Calling 'close' more than once on a single {@link Scope} instance


#nit

Suggested change

/// NOTE: Calling 'close' more than once on a single {@link Scope} instance

/// NOTE: Calling 'close' more than once on a single Scope instance

greglittlefield-wf · 2018-12-28T17:27:56Z

lib/src/abstract_scope_manager.dart

+  /// Returns a [Scope] instance to control the end of the active period for the
+  /// [Span]. It is a programming error to neglect calling [Scope].close() on
+  /// the returned instance.
+  Scope activate(Span span, {bool finishSpanOnClose});


I noticed that the spec lists finishSpanOnClose as required.

A required boolean parameter finish span on close will mark whether...

This should be made a positional parameter or have the @required annotation added.

I agree with your suggestion, but later came across this: opentracing/opentracing-java#291

I also noticed that your reference isn't the spec, but rather a suggested change to it. I'm inclined to remove this parameter for now, until that spec suggestion is accepted and/or we have an immediate use-case for it. Thoughts?

After creating an actual implementation in basictracer_dart, I've changed my position on your suggestion and think we should proceed with it.

greglittlefield-wf · 2018-12-28T17:10:03Z

lib/src/abstract_scope.dart

+  /// leads to undefined behavior.
+  void close();
+
+  /// Returns the [Span] that's been scoped by this [Scope]


#nit Getters should be documented using noun phrases.

Suggested change

/// Returns the [Span] that's been scoped by this [Scope]

/// The Span that's been scoped by this Scope.

I'll stop commenting on these; sorry, my OCD is kicking in... 😅

If you wouldn't be offended, I'd be more than happy to make a PR into this branch to update the doc comments to match the Dart style guide.

greglittlefield-wf · 2018-12-28T17:20:28Z

lib/src/abstract_tracer.dart

+
+  /// Returns the activer [Span]. This is a shorthand for
+  /// `Tracer.scopeManager().active().span()` and null will be returned if
+  /// [ScopeManager].active()} is null.


Based on the description, can we just provide a default implementation?

Span activeSpan() => scopeManager.active?.span;

greglittlefield-wf · 2018-12-28T23:05:15Z

lib/src/abstract_tracer.dart

+  /// Returns the activer [Span]. This is a shorthand for
+  /// `Tracer.scopeManager().active().span()` and null will be returned if
+  /// [ScopeManager].active()} is null.
+  Span activeSpan();


Are these changes missing the Tracer Changes from the spec?

Also, is there supposed to be an activeSpan on this class? I don't see that in the spec

Are these changes missing the Tracer Changes from the spec?

Yes, these changes are missing some portions of the proposed spec changes. The only parts I implemented were those that would specifically help accomplish the targeted use-case:
consolidate the spans occurring from the time the user has started to navigate to Home being loaded and populated.

Also, is there supposed to be an activeSpan on this class? I don't see that in the spec

Yes, this is a convenience method that was borrowed from https://developer.lightbend.com/docs/telemetry/current/extensions/opentracing/api.html#globaltracer-active-span and https://github.com/opentracing-contrib/java-agent#creating-custom-rules .
We could probably just include it in our implementation if we don't think we want it in this abstract class.

dustinlessard-wf · 2019-01-17T16:39:48Z

Ready for re-review.

greglittlefield-wf · 2019-01-28T20:03:54Z

lib/src/abstract_tracer.dart

  /// Returns the activer [Span]. This is a shorthand for
  /// `Tracer.scopeManager().active().span()` and null will be returned if
  /// [ScopeManager].active()} is null.
-  Span activeSpan();
+  Span activeSpan() => scopeManager.active?.span;


Now that I think about it, this should just be a getter.

greglittlefield-wf · 2019-01-28T21:18:13Z

lib/src/abstract_tracer.dart

@@ -54,6 +55,17 @@ abstract class AbstractTracer {
  ///     var serverSpan = Tracer.startSpan('...', { childOf : wireCtx });
  SpanContext extract(String format, dynamic carrier);

+  /// Returns the current [ScopeManager], which may be a noop but may not be
+  /// null.
+  ScopeManager get scopeManager;


Adding these abstract members is considered a breaking change, since classes that extend this will no longer implement them.

To avoid a major change, could we implement this and return a NoopScopeManager?

Good idea! That being said, I'm debating this and consulting with our Tracing people on it.

After consulting and debating the question of whether we should accept this getter, resulting in a breaking change, the following pros and cons were identified:

Pros:

The AbstractTracer continues to make it obvious what must be implemented by extenders/implementors.

We maintain the distinction between abstract classes and inert implementations.

Cons:

Propagation of this breaking change will require no less than 7 pull requests to update consumers. (mitigated via automation by RepoCommander)

Based on these considerations, I believe we should accept the breaking change and propagate it via RepoCommander.

greglittlefield-wf · 2019-01-28T21:18:21Z

lib/src/abstract_tracer.dart

+  /// null.
+  ScopeManager get scopeManager;
+
+  set scopeManager(ScopeManager value);


See scopeManager getter comment

greglittlefield-wf · 2019-01-28T21:18:57Z

lib/src/abstract_span.dart

@@ -37,6 +37,9 @@ abstract class Span {
  /// Each span has a UTC timestamp describing when it started.
  DateTime get startTime;

+  /// Each span has a UTC timestamp describing when it started.
+  set startTime(DateTime value);


Adding these abstract members is considered a breaking change, since classes that extend this will no longer implement them.

Do we need this change?

Yes, we need this change. Luckily at Workiva in Dart, we only use BasicTracer as our basis for all Tracing related classes.

greglittlefield-wf · 2019-01-28T21:23:31Z

example/default/json_serializable_span.dart

-  Map getFields() {
-    Map<String, dynamic> fieldMap = {};
+  Map<String, dynamic> getFields() {
+    Map<String, dynamic> fieldMap = <String, dynamic>{};


#nit you only need to specify generic parameters on the LHS or RHS, not both; Dart infers the rest for you!

I think the convention is to use the RHS, since it allows the omission of the full type on the LHS

Suggested change

Map<String, dynamic> fieldMap = <String, dynamic>{};

final fieldMap = <String, dynamic>{};

How can we avoid this?

That is from one of the lints opted into in this repo; I think that lint should be disabled unless there was a reason it was enabled.

Related: I agree with the sentiment expressed here by one of the linter package maintainers: dart-lang/linter#1068 (comment)

I do not believe that any project should enable a lint because there isn't a stated reason not to. I think, rather, that a project should only enable those lints for which there is a stated reason for it to be enabled.

greglittlefield-wf · 2019-01-28T21:26:31Z

test/unit/noop_tracer_test.dart

+  test('Verify startSpan returns NoopSpan from NoopScopeManager', () {
+    NoopTracer noOpTracer = new NoopTracer();
+    expect(
+        identical(noOpTracer.startSpan(''), noOpTracer.activeSpan()), isTrue);


#nit can use same matcher for a better error message

Suggested change

identical(noOpTracer.startSpan(''), noOpTracer.activeSpan()), isTrue);

same(noOpTracer.startSpan('')), noOpTracer.activeSpan());

ty! I didn't know that!

greglittlefield-wf · 2019-01-28T21:27:57Z

lib/src/noop_tracer.dart

  }

  @override
  Future<dynamic> flush() async {}
+
+  @override
+  Span activeSpan() {


Unnecessary override; this should use the same implementation as the abstract tracer, since this class seems to be the only thing providing activeSpan coverage.

dustinlessard-wf · 2019-02-04T15:20:49Z

Ready for review, fyi @greglittlefield-wf

greglittlefield-wf · 2019-02-05T17:04:42Z

lib/src/noop_scope.dart

+  /// Returns a new NoopScope, which will use the supplied Span. If no Span is
+  /// supplied, a static instance of a NoopSpan will be used.
+  NoopScope({Span span}) {
+    _span = span ?? _span;


Could overwriting the static _span when a new NoopScope is created have the potential to be problematic?

For instance:

final span1 = globalTracer().startSpan('span1'); final span2 = globalTracer().startSpan('span2'); final noopScope1 = noopScopeManager.activate(span1, false); print(noopScope1.span.operationName); // span1 final noopScope2 = noopScopeManager.activate(span2, false); print(noopScope1.span.operationName); // span2 - this shouldn't have changed, right? print(noopScope2.span.operationName); // span2

I would expect the implementation to look more like:

/// The No-op implementation of [Scope] in which all operations are no-op class NoopScope implements Scope { static final Span _noopSpan = new NoopSpan(); /// Returns a new NoopScope, which will use the supplied Span. If no Span is /// supplied, a static instance of a NoopSpan will be used. NoopScope({Span span}) : _span = span ?? _noopSpan; final Span _span; @override void close() {} @override Span get span => _span; }

greglittlefield-wf · 2019-02-05T17:06:24Z

lib/src/abstract_scope_manager.dart

+  /// Returns the [Scope], or null if none could be found.
+  Scope get active;
+
+  set active(Scope value);


There shouldn't be a setter here; this value gets set in activate, and there is no setter in the Java implementation.

Please see https://github.com/Workiva/basictracer-dart/pull/59/files#diff-7c439d7fe63b0d14ff72fcbc3d094df5R32 for the reason to include this setter.

This allows us to auto-nest scopes:

Span operationASpan = new BasicSpan('OperationA') scopeManager.activate(operationASpan, finishSpanOnClose); /// start Operation A /// make a bunch of spans representing sub-steps of Operation A Span operationA1 = new BasicSpan('operationA1'); scopeManager.activate(operationA1, finishSpanOnClose); /// make a bunch of spans representing sub-steps of Operation A1 /// end Operation A1 scopeManager.active.close() /// end Operation A scopeManager.active.close()

The resulting span structure would be similar to :

operationA |- spans for sub-steps |- operationA1 |-spans for sub-steps

Thoughts?

That makes sense. Could we document that the setter should only be used by Scope/ScopeManager implementations, and discourage using it otherwise?

We would have to add ignore comments wherever used, but I think it'd be worth it to annotate it as @protected as well.

greglittlefield-wf

+1, good stuff, @dustinlessard-wf!

sebastianmalysa-wf · 2019-02-06T18:40:59Z

QA +1

[x] CI passes

@Workiva/release-management-pp

rmconsole-wf added the Merge Requirements Unmet label Dec 15, 2018

AF-3505 : ActiveSpan

df1160c

dustinlessard-wf force-pushed the AF-3505 branch 2 times, most recently from d2f882a to df1160c Compare December 20, 2018 12:06

AF-3505: ci tweak

7a291a2

toddbeckman-wf reviewed Dec 20, 2018

View reviewed changes

sebastianmalysa-wf reviewed Dec 27, 2018

View reviewed changes

sebastianmalysa-wf approved these changes Dec 27, 2018

View reviewed changes

greglittlefield-wf mentioned this pull request Dec 28, 2018

Active span #33

Closed

greglittlefield-wf requested changes Dec 28, 2018

View reviewed changes

greglittlefield-wf reviewed Dec 28, 2018

View reviewed changes

greglittlefield-wf requested changes Jan 28, 2019

View reviewed changes

dustinlessard-wf force-pushed the AF-3505 branch from 38de148 to 2b26664 Compare February 1, 2019 14:37

AF-3505 : code cleanup

2802716

dustinlessard-wf force-pushed the AF-3505 branch from b2b3f2c to 2802716 Compare February 4, 2019 15:01

sebastianmalysa-wf approved these changes Feb 5, 2019

View reviewed changes

greglittlefield-wf requested changes Feb 5, 2019

View reviewed changes

AF-3505 : code cleanup

d3b40fd

greglittlefield-wf approved these changes Feb 6, 2019

View reviewed changes

sebastianmalysa-wf approved these changes Feb 6, 2019

View reviewed changes

rmconsole-wf added RM Ready and removed Merge Requirements Unmet labels Feb 6, 2019

rmconsole-wf added the Merge Requirements Met label Feb 6, 2019

rmconsole5-wk merged commit b508d73 into master Feb 6, 2019

rmconsole5-wk deleted the AF-3505 branch February 6, 2019 18:41

This was referenced Feb 7, 2019

Release opentracing_dart 0.2.5 #35

Closed

Release opentracing_dart 0.3.0 #36

Closed

Release opentracing_dart 1.0.0 #37

Merged

		class JsonSerializableSpan {
		// ignore: public_member_api_docs

	/// Many times a [Span] will be extant ([Span].finish() has not been called)
	/// Many times a [Span] will be extant ([Span.finish] has not been called)

		@@ -0,0 +1,21 @@
		import 'package:opentracing/opentracing.dart';

		/// A [Scope] formalizes the activation and deactivation of a [Span], usually

	/// A [Scope] formalizes the activation and deactivation of a [Span], usually
	/// A Scope formalizes the activation and deactivation of a [Span], usually

	/// but blocked on IO while the RPC is still outstanding. A [Scope] defines when
	/// but blocked on IO while the RPC is still outstanding. A Scope defines when

	/// NOTE: Calling 'close' more than once on a single {@link Scope} instance
	/// NOTE: Calling 'close' more than once on a single Scope instance

	/// Mark the end of the active period for the current thread and [Scope],
	/// Mark the end of the active period for the current thread and Scope,

	/// updating the [ScopeManager].active() in the process.
	/// updating the [ScopeManager.active] in the process.

	/// Returns the [Span] that's been scoped by this [Scope]
	/// The Span that's been scoped by this Scope.

	Map<String, dynamic> fieldMap = <String, dynamic>{};
	final fieldMap = <String, dynamic>{};

	identical(noOpTracer.startSpan(''), noOpTracer.activeSpan()), isTrue);
	same(noOpTracer.startSpan('')), noOpTracer.activeSpan());

AF-3505 : ActiveSpan #34

AF-3505 : ActiveSpan #34

Conversation

dustinlessard-wf commented Dec 15, 2018 • edited Loading

Description

Changes

Testing/QA

Code Review

aviary2-wf commented Dec 15, 2018

Security Insights

Action Items

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielharasymiw-wf commented Dec 20, 2018 • edited Loading

dustinlessard-wf commented Dec 21, 2018 • edited Loading

sebastianmalysa-wf Dec 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastianmalysa-wf left a comment

Choose a reason for hiding this comment

greglittlefield-wf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustinlessard-wf Dec 29, 2018 • edited Loading

Choose a reason for hiding this comment

dustinlessard-wf commented Jan 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustinlessard-wf Jan 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustinlessard-wf commented Feb 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustinlessard-wf Feb 5, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greglittlefield-wf left a comment

Choose a reason for hiding this comment

sebastianmalysa-wf commented Feb 6, 2019

dustinlessard-wf commented Dec 15, 2018 •

edited

Loading

danielharasymiw-wf commented Dec 20, 2018 •

edited

Loading

dustinlessard-wf commented Dec 21, 2018 •

edited

Loading

sebastianmalysa-wf Dec 27, 2018 •

edited

Loading

dustinlessard-wf Dec 29, 2018 •

edited

Loading

dustinlessard-wf Jan 29, 2019 •

edited

Loading

dustinlessard-wf Feb 5, 2019 •

edited

Loading