Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Step 1 - Use new Activity to Replace OT Span #660

Merged
merged 19 commits into from
May 13, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion NuGet.config
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<configuration>
<packageSources>
<clear />
<add key="NuGet" value="https://api.nuget.org/v3/index.json" />
<add key="LocalDiagSource" value="C:\repos\tempnuget" />
</packageSources>
<disabledPackageSources />
</configuration>
12 changes: 10 additions & 2 deletions samples/Exporters/Console/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class Program
/// <param name="args">Arguments from command line.</param>
public static void Main(string[] args)
{
Parser.Default.ParseArguments<JaegerOptions, ZipkinOptions, ApplicationInsightsOptions, PrometheusOptions, HttpClientOptions, StackdriverOptions, LightStepOptions, ZPagesOptions, ConsoleOptions, OtlpOptions>(args)
Parser.Default.ParseArguments<JaegerOptions, ZipkinOptions, ApplicationInsightsOptions, PrometheusOptions, HttpClientOptions, StackdriverOptions, LightStepOptions, ZPagesOptions, ConsoleOptions, ConsoleActivityOptions, OtlpOptions>(args)
.MapResult(
(JaegerOptions options) => TestJaeger.Run(options.Host, options.Port),
(ZipkinOptions options) => TestZipkin.Run(options.Uri),
Expand All @@ -50,6 +50,7 @@ public static void Main(string[] args)
(LightStepOptions options) => TestLightstep.Run(options.AccessToken),
(ZPagesOptions options) => TestZPages.Run(),
(ConsoleOptions options) => TestConsole.Run(options),
(ConsoleActivityOptions options) => TestConsoleActvity.Run(options),
(OtlpOptions options) => TestOtlp.Run(options.Endpoint),
errs => 1);

Expand Down Expand Up @@ -127,7 +128,14 @@ internal class ZPagesOptions
[Verb("console", HelpText = "Specify the options required to test console exporter")]
internal class ConsoleOptions
{
[Option('p', "pretty", HelpText = "Specify if the output should be pretty printed (default: false)", Default = false)]
[Option('p', "pretty", HelpText = "Specify if the output should be pretty printed (default: false)", Default = true)]
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
public bool Pretty { get; set; }
}

[Verb("consoleactivity", HelpText = "Specify the options required to test console activity exporter")]
internal class ConsoleActivityOptions
{
[Option('p', "pretty", HelpText = "Specify if the output should be pretty printed (default: false)", Default = true)]
public bool Pretty { get; set; }
}

Expand Down
47 changes: 47 additions & 0 deletions samples/Exporters/Console/TestConsoleActivity.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// <auto-generated/>
cijothomas marked this conversation as resolved.
Show resolved Hide resolved

using OpenTelemetry.Trace.Configuration;
using OpenTelemetry.Exporter.Console;
using OpenTelemetry.Trace;
using System.Diagnostics;
using OpenTelemetry.Trace.Export;

namespace Samples
{
internal class TestConsoleActvity
{
internal static object Run(ConsoleActivityOptions options)
{
// Setup exporter
var exporterOptions = new ConsoleExporterOptions
{
Pretty = options.Pretty
};
var activityExporter = new ConsoleActivityExporter(exporterOptions);

// Setup processor
var activityProcessor = new SimpleActivityProcessor(activityExporter);

// Enable OpenTelemetry for the "source" named "DemoSource".
OpenTelemetrySDK.EnableOpenTelemetry("DemoSource", activityProcessor);

// Everything above this line is required only in Applications
// which decide to use OT.

// The following is generating activity.
// A library would simply write the following line of code.
var source = new ActivitySource("DemoSource");
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
using (var parent = source.StartActivity("parent"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is parent end up being display name or "enumeration" name?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not a display name, please add code that sets the display name

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StartActivity("anythinghere") - "anythinghere" becomes OperationName, DisplayName by default. DisplayName can be overridden, but OperationName cannot be.

I think the intention is DisplayName to become OT SpanName, as OT allows changing it after starting, but Activity.OperationName is fixed at creation.

{
parent?.AddTag("parent location", "parent location");
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are integers and other types allowed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only string - this is a drift from OT.


using (var child = source.StartActivity("child"))
{
child?.AddTag("child location", "child location");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add example with the result setting. I bet the most of implementation will need try{}catch to indicate that the activity failed

}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If parent may be null (see parent?.), will the using statement fail?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Parent will be null if none is listening.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be a real pain, many many people will forgot the null check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To sergey's qn - the using statement doesn't fail.
But any other place where parent is not nullchecked will fail. Hence the parent? usage throughout. Users are expected to do this, and this is unit testable easily.


return null;
}
}
}
2 changes: 1 addition & 1 deletion src/OpenTelemetry.Api/OpenTelemetry.Api.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
</PropertyGroup>
-->
<ItemGroup>
<PackageReference Include="System.Diagnostics.DiagnosticSource" Version="4.6.0" />
<PackageReference Include="System.Diagnostics.DiagnosticSource" Version="5.0.0-dev" />
</ItemGroup>

</Project>
58 changes: 58 additions & 0 deletions src/OpenTelemetry.Exporter.Console/ConsoleActivityExporter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// <copyright file="ConsoleActivityExporter.cs" company="OpenTelemetry Authors">
// Copyright 2018, OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

using System.Collections.Generic;
using System.Diagnostics;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading;
using System.Threading.Tasks;
using OpenTelemetry.Trace.Export;

namespace OpenTelemetry.Exporter.Console
{
public class ConsoleActivityExporter : ActivityExporter
{
private readonly JsonSerializerOptions serializerOptions;

public ConsoleActivityExporter(ConsoleExporterOptions options)
{
this.serializerOptions = new JsonSerializerOptions
{
WriteIndented = options.Pretty,
};

this.serializerOptions.Converters.Add(new JsonStringEnumConverter());
this.serializerOptions.Converters.Add(new ActivitySpanIdConverter());
this.serializerOptions.Converters.Add(new ActivityTraceIdConverter());
}

public override Task<ExportResult> ExportAsync(IEnumerable<Activity> batch, CancellationToken cancellationToken)
{
foreach (var span in batch)
{
System.Console.WriteLine(JsonSerializer.Serialize(span, this.serializerOptions));
}

return Task.FromResult(ExportResult.Success);
}

public override Task ShutdownAsync(CancellationToken cancellationToken)
{
return Task.CompletedTask;
}
}
}
1 change: 1 addition & 0 deletions src/OpenTelemetry/OpenTelemetry.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

<ItemGroup>
<PackageReference Include="System.Collections.Immutable" Version="1.4.0" />
<PackageReference Include="System.Diagnostics.DiagnosticSource" Version="5.0.0-dev" />
<PackageReference Include="System.Reflection.Emit.Lightweight" Version="4.7.0" />
</ItemGroup>

Expand Down
40 changes: 40 additions & 0 deletions src/OpenTelemetry/Trace/Configuration/OpenTelemetrySDK.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// <copyright file="OpenTelemetrySDK.cs" company="OpenTelemetry Authors">
// Copyright 2018, OpenTelemetry Authors
SergeyKanzhelev marked this conversation as resolved.
Show resolved Hide resolved
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

using System.Diagnostics;
using OpenTelemetry.Trace.Export;

namespace OpenTelemetry.Trace.Configuration
{
public class OpenTelemetrySDK
bruno-garcia marked this conversation as resolved.
Show resolved Hide resolved
{
static OpenTelemetrySDK()
{
Activity.DefaultIdFormat = ActivityIdFormat.W3C;
Activity.ForceDefaultIdFormat = true;
}

public static void EnableOpenTelemetry(string source, ActivityProcessor activityProcessor)
{
ActivitySource.AddActivityListener(
activitySource => activitySource.Name.Equals(source),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, indent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cijothomas crossing this with dotnet/runtime#35326 I can understand the parameters but it will be good to name the parameters or add comments here, especially for the second and third parameters. Anyway, the signatures are already better per https://source.dot.net/#System.Diagnostics.DiagnosticSource/System/Diagnostics/ActivityListener.cs,42a5d087d90a1839,references

(activitySource, name, kind, context, tags, links) => ActivityDataRequest.AllData,
(activitySource, name, kind, parentId, tags, links) => ActivityDataRequest.AllData,
activityProcessor.OnStart,
activityProcessor.OnEnd);
}
}
}
61 changes: 61 additions & 0 deletions src/OpenTelemetry/Trace/Export/ActivityExporter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// <copyright file="ActivityExporter.cs" company="OpenTelemetry Authors">
// Copyright 2018, OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>
using System.Collections.Generic;
using System.Diagnostics;
using System.Threading;
using System.Threading.Tasks;

namespace OpenTelemetry.Trace.Export
{
/// <summary>
/// ActivityExporter base class.
/// </summary>
public abstract class ActivityExporter
{
public enum ExportResult
{
/// <summary>
/// Batch is successfully exported.
/// </summary>
Success = 0,

/// <summary>
/// Batch export failed. Caller must not retry.
/// </summary>
FailedNotRetryable = 1,

/// <summary>
/// Batch export failed transiently. Caller should record error and may retry.
/// </summary>
FailedRetryable = 2,
}

/// <summary>
/// Exports batch of activities asynchronously.
/// </summary>
/// <param name="batch">Batch of activities to export.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>Result of export.</returns>
public abstract Task<ExportResult> ExportAsync(IEnumerable<Activity> batch, CancellationToken cancellationToken);

/// <summary>
/// Shuts down exporter asynchronously.
/// </summary>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>Returns <see cref="Task"/>.</returns>
public abstract Task ShutdownAsync(CancellationToken cancellationToken);
}
}
46 changes: 46 additions & 0 deletions src/OpenTelemetry/Trace/Export/ActivityProcessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// <copyright file="ActivityProcessor.cs" company="OpenTelemetry Authors">
// Copyright 2018, OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>
using System.Diagnostics;
using System.Threading;
using System.Threading.Tasks;

namespace OpenTelemetry.Trace.Export
{
/// <summary>
/// Activity processor base class.
/// </summary>
public abstract class ActivityProcessor
{
/// <summary>
/// Span Activity hook.
/// </summary>
/// <param name="activity">Instance of span to process.</param>
public abstract void OnStart(Activity activity);

/// <summary>
/// Span Activity hook.
/// </summary>
/// <param name="activity">Instance of Span to process.</param>
public abstract void OnEnd(Activity activity);

/// <summary>
/// Shuts down Activity processor asynchronously.
/// </summary>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>Returns <see cref="Task"/>.</returns>
public abstract Task ShutdownAsync(CancellationToken cancellationToken);
}
}
Loading