Distributed Tracing #261

TsuyoshiUshio · 2019-03-05T03:25:15Z

I'm creating this pull request to have this branch to hands off to you.
I open this PR for visibility.

What I've done:

Correlation design for Orchestration and implementation
Working Sample

TODO

Error Tracking for throwing exception case.
Continuous as New design
Durable Functions Extension contribution

Now one of the test case will fail for not implementing it for multiple orchestration retry case. However, I open this for share.

TsuyoshiUshio · 2019-03-08T03:31:21Z

Done

Correlation with Orchestration
Correlation with Multiple Orchestration
Correlation with Exception
Correlation with Retry (Activity)
Correlation with Retry (Orchestrator)
Multi Layered Orchestration
Multi Layered Orchestration with Retry
Fan-Out Fan-In

TODO

Continue As New Testing
Automatic tracked Dependency with correlation
Correlation with Http Correlation Protocol ( Currently only for W3C TraceContext)
Activity Restoration (After .NET implement it)

cgillum · 2019-03-09T19:57:37Z

src/DurableTask.Core/TaskOrchestration.cs

@@ -132,7 +132,7 @@ public override string GetStatus()
        /// <param name="context">The orchestration context</param>
        /// <param name="input">The typed input</param>
        /// <returns>The typed output</returns>
-        public abstract Task<TResult> RunTask(OrchestrationContext context, TInput input);
+        public abstract Task<TResult> RunTaskAsync(OrchestrationContext context, TInput input);


This is a breaking change, which cannot be allowed.

Hi @cgillum

Sorry, I didn't notice that. However, I might mistake to refactor the name. I recover it.

TsuyoshiUshio · 2019-03-10T00:07:44Z

Hi @cgillum The CI fails, it looks the script can't refer the $ServiceBusConnectionString . Hmm I have no idea. Any thoughts? If not, I'll ask the Azure DevOps guys.

cgillum · 2019-03-10T00:16:14Z

@TsuyoshiUshio I'm not sure. The CI passed earlier today for a different PR. I have not made any changes to the pipeline.

TsuyoshiUshio · 2019-03-10T01:04:05Z

@cgillum I'm asking the Azure DevOps guys.

simonporter · 2019-03-11T23:56:33Z

Correlation.Sample.Tests/StringExtensionsTest.cs

@@ -0,0 +1,38 @@
+using Microsoft.VisualStudio.TestTools.UnitTesting;


using [](start = 0, length = 5)

File header + move using into namespace

Looks like this still needs to be done?

simonporter · 2019-03-11T23:58:07Z

src/DurableTask.Core/TaskActivityWorkItem.cs

@@ -11,6 +11,8 @@
 //  limitations under the License.
 //  ----------------------------------------------------------------------------------

+using System.Diagnostics;


using [](start = 0, length = 5)

move into namespace

simonporter · 2019-03-11T23:58:41Z

src/DurableTask.Core/CorrelationTraceContext.cs

+    {
+
+#if NETSTANDARD2_0
+        private static AsyncLocal<TraceContext> current = new AsyncLocal<TraceContext>();


private [](start = 8, length = 7)

don't use private modifier per rest of repo convention

Hi @simonporter ,
Before fix this, Could you clarify the convention? Should I use internal or default for the modifier in this case? Why do you forbid private modifier? I'd like to know the reason for the future contribution of this repo. Also want to properly fix this.

Done that. I change private into default.

TsuyoshiUshio · 2019-03-13T18:04:40Z

One note. I'm adding Microsoft.ApplicationInsights namespace on the DurableTask.Core. It is just temporary. It is used for ActivityExtension which enable W3C TraceContext. Application Insights team move the extension to the System.Diagnostics namespace. The PR is already merged 15 days before. It will be included the next version of the .Net Core implementation. dotnet/corefx#33207

TsuyoshiUshio · 2019-03-14T22:16:39Z

Hi @simonporter ,

Sounds good for the convention? I'm working with @cgillum for enabling Distributed Tracing for Durable Functions. However, we try to make it available for other teams that use DurableTask. This PR is not definitive version. If you don't like my implementation, you can dispose it. However, the architecture and what I experienced might be helpful at least for the future implementor. Could you give me 30 min to explain this PR in person? I'll come. (Hopefully with @cgillum . ) This PR might be too big for review in here.

TsuyoshiUshio · 2019-03-14T22:35:55Z

The CI fails are the CI issue. Not code. the lack of permission for public PR.
I'm investigating it with my colleague.

ShreyasRmsft · 2019-03-19T09:53:40Z

@TsuyoshiUshio thank you for bringing this to our notice. We (VsTest team) are working on getting distributed test runs to work with public projects. Will keep you updated.

TsuyoshiUshio · 2019-03-19T15:26:37Z

Hi @ShreyasRmsft , Thank you for telling me! Sure. Sounds exciting. until then we just add PR contributors some kind of group.

TsuyoshiUshio · 2019-03-19T15:41:08Z

@ShreyasRmsft Could you tell me the best place to file this issue? I spent several days for this issue. I don't want people have the same experience as me. Also, if it is OK. could you tell me when it is going to be published? I also MSFT guy. However, I can't identify you from your GitHub account. :)

ShreyasRmsft · 2019-03-20T06:02:18Z

@TsuyoshiUshio you can raise the issue on https://github.com/Microsoft/azure-pipelines-tasks/issues

TsuyoshiUshio · 2019-03-20T14:44:33Z

Thank you, @ShreyasRmsft , I've done that!

TsuyoshiUshio · 2019-03-21T18:08:13Z

Hi @simonporter

This is the sample of the raw message with Two Layered OrchestratorTraceContext. Please see the TraceContextStore part. I change the serialization option only for this part for supporting reference. I don't want to change the whole serialization option. Only for TraceContextStore part. However, if you have any idea to reduce the message or put out of queue, please let me know. around 1500 bytes.

{"$type":"DurableTask.AzureStorage.MessageData","ActivityId":"76185f4d-bb4a-4ed4-857e-3e9eb1e42690","TaskMessage":{"$type":"DurableTask.Core.TaskMessage","Event":{"$type":"DurableTask.Core.History.TaskCompletedEvent","EventType":5,"TaskScheduledId":0,"Result":"\"Hello world with retry\"","EventId":-1,"IsPlayed":false,"Timestamp":"2019-03-21T17:48:43.5263421Z"},"SequenceNumber":0,"OrchestrationInstance":{"$type":"DurableTask.Core.OrchestrationInstance","InstanceId":"67b1704f68b049ae9d415d58aa0169c9:2","ExecutionId":"163b7b9a6efe40d586a68f844f7461b4"}},"CompressedBlobName":null,"SequenceNumber":9,"Episode":1,"TraceContextStore":{"$type":"DurableTask.Core.TraceContextStore","TraceContextJson":"{\"$id\":\"1\",\"$type\":\"DurableTask.Core.TraceContext, DurableTask.Core\",\"StartTime\":\"2019-03-21T17:48:43.526875+00:00\",\"ParentId\":\"|568a23ca-42c37626d2275a9f.568a23cf_568a23fc_568a2451_568a2456_568a2458_\",\"Traceparent\":\"00-c91fda74588ce04693a54a2ce6919c56-bab42c068a951740-02\",\"Tracestate\":null,\"ParentSpanId\":\"0463ebaf65d93843\",\"OrchestrationTraceContexts\":[{\"$id\":\"2\",\"$type\":\"DurableTask.Core.TraceContext, DurableTask.Core\",\"StartTime\":\"2019-03-21T17:47:26.5219035+00:00\",\"ParentId\":\"|568a23ca-42c37626d2275a9f.568a23cf_568a23fc_\",\"Traceparent\":\"00-c91fda74588ce04693a54a2ce6919c56-2158e5eba664fb4a-02\",\"Tracestate\":null,\"ParentSpanId\":\"cf0bcc1ba2f5f34f\",\"OrchestrationTraceContexts\":[{\"$ref\":\"2\"}],\"RootId\":\"00-c91fda74588ce04693a54a2ce6919c56-2158e5eba664fb4a-02\"},{\"$id\":\"3\",\"$type\":\"DurableTask.Core.TraceContext, DurableTask.Core\",\"StartTime\":\"2019-03-21T17:48:43.4696347+00:00\",\"ParentId\":\"|568a23ca-42c37626d2275a9f.568a23cf_568a23fc_568a2451_\",\"Traceparent\":\"00-c91fda74588ce04693a54a2ce6919c56-d84f945c3a626642-02\",\"Tracestate\":null,\"ParentSpanId\":\"2158e5eba664fb4a\",\"OrchestrationTraceContexts\":[{\"$ref\":\"2\"},{\"$ref\":\"3\"}],\"RootId\":\"00-c91fda74588ce04693a54a2ce6919c56-d84f945c3a626642-02\"}],\"RootId\":\"00-c91fda74588ce04693a54a2ce6919c56-bab42c068a951740-02\"}"}}

TsuyoshiUshio · 2019-03-28T21:33:52Z

Hi @simonporter , @cgillum ,

I implement the test for ContinueAsNew. The current behavior is that Orchestration which is created by ContinueAsNew is considered as the same Orchestration. It is because of the isReplay is true after the ContinueAsNew. If you don't like this behavior, we might change the behavior that if an orchestration is continueAsNew, then start a new orchestration. However, current behavior looks good as well.

adarsh1 · 2019-03-30T00:20:34Z

src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs

@@ -612,6 +614,7 @@ static TaskHubInfo GetTaskHubInfo(string taskHub, int partitionCount)
                    session.StartNewLogicalTraceScope();

                    List<MessageData> outOfOrderMessages = null;
+                    var current = Activity.Current; // Correlation for checking the state of the Activity.Current. TODO remove


Is this still needed?

adarsh1 · 2019-04-14T22:43:52Z

src/DurableTask.Core/TraceContext.cs

+//  ----------------------------------------------------------------------------------
+
+namespace DurableTask.Core
+{


As discussed could this be refactored to encapsulate activity, thus reducing the need to decide between both?

I introduce the new structure. I encapsulate activity to the TraceContext object. New name is

TraceContextBase (abstract class)

W3CTraceContext (implementation of the TraceContextBase)

HttpCorrelationProtocolTraceContext (Implementation of the TraceContextBase)

For enabling this, I create a Factory to refer the configuration of the Protocol settings.
These two Contexts have limited property. Some property is not serialized to reduce the size of the message. It requires special serialization, It is handled on TraceContextStore

adarsh1 · 2019-04-14T22:45:16Z

src/DurableTask.Core/TraceMessages.cs

+{
+    /// <summary>
+    /// TraceMessages for Distributed Tracing
+    /// </summary>


Should this be a static class? also might make sense to rename to TraceConstants.

adarsh1 · 2019-04-14T22:58:35Z

src/DurableTask.Core/TraceContextStore.cs

+    /// For the Newton.json 7.0.1 requires JsonSerializerSettings. It will all so requires DurableTask users.
+    /// To avoid this breaking change, I provide TraceContextStore which support the serialization/deserialization of the <see cref="TraceContext"/>
+    /// </summary>
+    public class TraceContextStore


Since this is only used while storing and retrieving from Message Data, would it make sense to handle this simply as a property and constructor on Trace Context? We could also just use a custom JsonConverter attribute on top of the TraceContext since it seems like it needs to be serialized in a special way? we could also handle it via a private JsonProperty on message data. Incase neither of these are possible consider renaming to maybe SerializableTraceContext?

I removed the TraceContextStore and make it available for TraceContextBase. I implement it as property and static method. I tried to use constructor, however, it requires additional logic to move restored object to the target object. It could be error prone. I create a Restore method simply pass the json string it creates TraceContextBase subclass object.

adarsh1 · 2019-04-14T23:01:49Z

src/DurableTask.Core/TaskOrchestrationContext.cs

@@ -341,6 +341,11 @@ public void HandleTaskFailedEvent(TaskFailedEvent failedEvent)
                var taskFailedException = new TaskFailedException(failedEvent.EventId, taskId, info.Name, info.Version,
                    failedEvent.Reason, cause);

+                // correlation
+                var traceContext = CorrelationTraceContext.Current;


are traceContext and activity used?

adarsh1 · 2019-04-14T23:31:56Z

src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs

+            if (orchestrationState.OrchestrationStatus == OrchestrationStatus.Completed)
+            {
+                CorrelationTraceClient.TrackDepencencyTelemetry(dependencyActivity);
+            }


Why are these all separate Ifs? instead of a single if ((orchestrationState.OrchestrationStatus == OrchestrationStatus.Completed) ||
(orchestrationState.OrchestrationStatus == OrchestrationStatus.Failed)) ?

adarsh1 · 2019-04-14T23:51:40Z

src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs

                Guid traceActivityId = Guid.NewGuid();
                var session = new ActivitySession(this.storageAccountName, this.settings.TaskHubName, message, traceActivityId);
                session.StartNewLogicalTraceScope();
+
+                // correlation 
+                string name = $"{TraceMessages.Activity} {((TaskScheduledEvent)session.MessageData.TaskMessage.Event)?.Name?.GetTargetClassName()}";


@simonporter to confirm here, but I don't believe this custom logic to get the class NAme would always hold, particularly in the case of a custom NameVersion object manager. Can this be moved one level up to The TaskOrchestrationManager in Core? In that case you would have access to the object manager and can get the class NAme more simply.

@adarsh1 Where can I find the TaskOrchestrationManager? I can't find it.

Sorry meant the TaskActivityDispatcher

adarsh1 · 2019-04-14T23:54:06Z

src/DurableTask.AzureStorage/Messaging/TaskHubQueue.cs

+                    currentTraceContext = currentTraceContext ?? current.CreateTraceContext();
+
+                    data.TraceContextStore = TraceContextStore.Create(currentTraceContext);
+                    rawContent = await messageManager.SerializeMessageDataAsync(data);


looks like data is the only thing changing between the if and else, should the raw content initialization simply stay out of this if else structure?

Test/DurableTask.AzureStorage.Tests/DurableTask.AzureStorage.Tests.csproj

adarsh1 · 2019-04-14T23:56:57Z

src/DurableTask.AzureStorage/DurableTask.AzureStorage.csproj

@@ -35,4 +35,5 @@
  <ItemGroup>
    <ProjectReference Include="..\DurableTask.Core\DurableTask.Core.csproj" />
  </ItemGroup>
+


Undo if no change.

TsuyoshiUshio · 2019-04-15T00:11:55Z

Thank you @adarsh1 ! I'll do it as we discussed. I'll start this week. :) I appreciate your review.

TsuyoshiUshio · 2019-05-06T06:59:32Z

Hi @adarsh1 , @cgillum

I fix all your request. Review it again, please. I checked the Scenario Test passes and execute the sample and check if we can see the same tracing on the portal.

TODO

Check if there is no affect for ServiceBus side. (even if they don't implement Distributed Tracing) using CI (We might add my github e-mail account to the CI to validate)
Rebase (I haven't done rebase for current master to keep commit log for review. once it becomes ok, I can rebase it and solve conflict (if there is ))
Library Project(DurableTaskCorrelationTelemetryInitializer and TraceContextBaseExtensions should be shared. however, I'm not sure where to share since it references Application Insights. Maybe DurableTask.Correlation or something?
More Unit Testing for TraceContextBase and sub classes.

adarsh1 · 2019-05-10T18:34:23Z

src/DurableTask.Core/TraceContextBase.cs

+        /// This property is not serialized.
+        /// </summary>
+        [JsonIgnore]
+        public Activity CurrentActivity { get; set; } // TODO make it protected after refactoring is done. 


make protected, or remove TODO

I double check the dependency. This property depend on the subclass, factory, and test class. I hope only these three can use this property. I madeit internal and removed TODO.

adarsh1 · 2019-05-10T18:36:05Z

src/DurableTask.Core/W3CTraceContext.cs

+            Traceparent = CurrentActivity.GetTraceparent();
+            Tracestate = CurrentActivity.GetTracestate();
+            ParentSpanId = CurrentActivity.GetParentSpanId();
+            // ParentId = activity.Id, // TODO check if it is not necessary


Is this still necessary?

adarsh1 · 2019-05-10T18:37:40Z

src/DurableTask.Core/HttpCorrelationProtocolTraceContext.cs

+        public override TimeSpan Duration => CurrentActivity?.Duration ?? DateTimeOffset.UtcNow - StartTime;
+
+        /// <inheritdoc />
+        public override string TelemetryId => throw new NotImplementedException(); // TODO Implement the HTTPCorrelation Protocol


Is this Safe? Would things fail if HttpCorrelationProtocol is used?

HttpCorrelationProtocol is not fully tested. App Insights team encourage to use W3C now. So I start implement/test with W3C. However, I implement HttpCorrelationProtocol. It should work, however, to make sure it works, it requires Scenario Test with TelemetryInitializer for HttpCorrelationProtocol.

adarsh1 · 2019-05-10T18:48:17Z

src/DurableTask.Core/TraceContextBase.cs

+        public string SerializableTraceContext =>
+            JsonConvert.SerializeObject(this, new JsonSerializerSettings()
+            {
+                TypeNameHandling = TypeNameHandling.Objects,


We really should be using a custom JsonConverter here, if we need special handling.
If we don't have the time to do that for now maybe a TODO would be ok?

I add TODO.

adarsh1 · 2019-05-10T18:57:48Z

src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs

                Guid traceActivityId = Guid.NewGuid();
                var session = new ActivitySession(this.storageAccountName, this.settings.TaskHubName, message, traceActivityId);
                session.StartNewLogicalTraceScope();
+
+                // correlation 
+                string name = $"{TraceMessages.Activity} {((TaskScheduledEvent)session.MessageData.TaskMessage.Event)?.Name?.GetTargetClassName()}";


Sorry meant the TaskActivityDispatcher

adarsh1

ad a few more nits around existing TODOs, other than that looks ok to me

TsuyoshiUshio · 2019-05-18T05:44:39Z

Thank you for your review, @adarsh1 , Hi, @cgillum , can I add my github account to the Azure DevOps task? I'd like to test this PR doesn't broke tests exists.

…the current users.

TsuyoshiUshio

Fix the requests

TsuyoshiUshio · 2020-01-18T23:09:32Z

Hi @cgillum ,

I fix the issues that you requested. However, some need to discuss with you. I leave these as unresovled comments. Could you have a look these three?

TelmetryActiviator's TelemetryClient should be static, left a comment.
samples/Correlation.Samples/Program.cs I leave the comments for represent the available demo scenario.
How? (following picture)

src/DurableTask.Core/StackExtensions.cs

cgillum · 2020-01-20T19:35:09Z

Hi @TsuyoshiUshio - I have reviewed your latest updates and responded to your questions. There are also a few requested fixes that have not been made yet. Please take a look.

… member.

TsuyoshiUshio

fix issues.

…ore.

TsuyoshiUshio · 2020-01-22T01:03:39Z

Hi @cgillum , I fixed them. Only one concerns left.

I fixed static TelemetryClient issue and find out the root cause. I'd like to listen the opinion about the design decision. Please refer the comment. (I leave it not resolved)

TsuyoshiUshio · 2020-01-27T18:39:06Z

Hi @cgillum this is friendly reminder. :) Please have a look. If you are ok, let's merge it.
If not, I'd happy to fix or refactor some parts.

jkerak · 2020-01-30T16:41:11Z

Hi. I'm not sure if this is the correct place to ask this question (I can't find a link to some kind of issue or feature request). Is there any kind of limit to number of executions that will be logged (and correlated) to AI? In some cases, I may have tens of thousands of activity function executions within a single orchestrator execution. I'm also curious what an eternal orchestrator would look like. To be clear, I'm really asking about what the limit of the app insights UI might be. I'm sure log analytics would store whatever is sent to it.

cgillum

OK - thanks for your patience. I suggest one more small change and then let's merge this.

src/DurableTask.Core/CorrelationTraceClient.cs

TsuyoshiUshio · 2020-01-30T19:17:49Z

Hi @cgillum I fixed all of the issues.

cgillum · 2020-01-30T20:06:02Z

Thanks! I will merge this now. Glad to get this first step finally checked in. :)

TsuyoshiUshio · 2020-01-30T20:55:34Z

Hi @jkerak

You are using tens of thousands of activity functions in a single orchestrator right? I'm not sure about the limitation of the durable side, however, Application Insights has some quota https://docs.microsoft.com/en-us/azure/azure-monitor/app/api-custom-events-metrics#limits This issue might be a good place to talk about correlation. Azure/azure-functions-durable-extension#939

TsuyoshiUshio · 2020-01-30T20:57:00Z

Thank you @cgillum for kind support of this PR. I'll go in implementing durable functions side.

cgillum changed the title ~~Distributed Tracing Hands Off~~ Distributed Tracing Mar 7, 2019

cgillum added durable functions This issue is an ask from the Durable Functions team. enhancement labels Mar 7, 2019

cgillum reviewed Mar 9, 2019

View reviewed changes

TsuyoshiUshio force-pushed the correlation/refactor branch 2 times, most recently from 6b2c65b to f3d15b3 Compare March 9, 2019 23:35

simonporter reviewed Mar 11, 2019

View reviewed changes

TsuyoshiUshio mentioned this pull request Mar 19, 2019

Multi agent Test fails with Public repo with GitHub PR Trigger microsoft/azure-pipelines-tasks#9879

Closed

adarsh1 reviewed Apr 14, 2019

View reviewed changes

adarsh1 reviewed May 10, 2019

View reviewed changes

TsuyoshiUshio added 8 commits January 18, 2020 12:23

Remove old config.

43aef10

Revert comment for not related correlation

d29c14a

Add reason for TelemetryClient is static on TelemetryActivator

df42249

Remove unnecessary pragma

5442fc2

Change the class name for representing responsiblity.

26f13be

Add comment for feature removal ApplicationInsights.DependencyCollector

c1ef442

Add CorrelationTraceClient.Propagate for remove addigional trace for …

787568c

…the current users.

cache the JsonSerializer

de5773b

TsuyoshiUshio commented Jan 18, 2020

View reviewed changes

cgillum reviewed Jan 20, 2020

View reviewed changes

src/DurableTask.Core/StackExtensions.cs Outdated Show resolved Hide resolved

TsuyoshiUshio added 4 commits January 21, 2020 16:14

Revert contextBase to context when it is not related with correlation

3fece2f

Revert messageManager to this.messageManager

8e8ad0e

follow the repository standard.

9db8fdd

fix issue of getting error if I change TelemetryClient to an instance…

adf9587

… member.

TsuyoshiUshio commented Jan 22, 2020

View reviewed changes

Fix the issue of hidden dependency of DurableTask.AzureStorage from C…

1328949

…ore.

TsuyoshiUshio requested a review from cgillum January 30, 2020 15:30

cgillum approved these changes Jan 30, 2020

View reviewed changes

src/DurableTask.Core/CorrelationTraceClient.cs Outdated Show resolved Hide resolved

Make it static since there is no instance method on this class.

2896655

cgillum merged commit 13b6d4c into Azure:correlation Jan 30, 2020

TsuyoshiUshio added a commit to TsuyoshiUshio/durabletask that referenced this pull request Mar 21, 2020

Distributed Tracing (Azure#261)

7365422

		@@ -0,0 +1,38 @@
		using Microsoft.VisualStudio.TestTools.UnitTesting;

Distributed Tracing #261

Distributed Tracing #261

Conversation

TsuyoshiUshio commented Mar 5, 2019

What I've done:

TODO

TsuyoshiUshio commented Mar 8, 2019

Done

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TsuyoshiUshio commented Mar 10, 2019

cgillum commented Mar 10, 2019

TsuyoshiUshio commented Mar 10, 2019

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

TsuyoshiUshio commented Mar 13, 2019

TsuyoshiUshio commented Mar 14, 2019

TsuyoshiUshio commented Mar 14, 2019

ShreyasRmsft commented Mar 19, 2019

TsuyoshiUshio commented Mar 19, 2019

TsuyoshiUshio commented Mar 19, 2019

ShreyasRmsft commented Mar 20, 2019

TsuyoshiUshio commented Mar 20, 2019

TsuyoshiUshio commented Mar 21, 2019

TsuyoshiUshio commented Mar 28, 2019

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

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

TsuyoshiUshio commented Apr 15, 2019

TsuyoshiUshio commented May 6, 2019 • edited Loading

TODO

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

adarsh1 left a comment

Choose a reason for hiding this comment

TsuyoshiUshio commented May 18, 2019

TsuyoshiUshio left a comment

Choose a reason for hiding this comment

TsuyoshiUshio commented Jan 18, 2020

cgillum commented Jan 20, 2020

TsuyoshiUshio left a comment

Choose a reason for hiding this comment

TsuyoshiUshio commented Jan 22, 2020

TsuyoshiUshio commented Jan 27, 2020 • edited Loading

jkerak commented Jan 30, 2020 • edited Loading

cgillum left a comment

Choose a reason for hiding this comment

TsuyoshiUshio commented Jan 30, 2020

cgillum commented Jan 30, 2020

TsuyoshiUshio commented Jan 30, 2020

TsuyoshiUshio commented May 6, 2019 •

edited

Loading

TsuyoshiUshio commented Jan 27, 2020 •

edited

Loading

jkerak commented Jan 30, 2020 •

edited

Loading