Skip to content

Commit

Permalink
feat: Fail fast when insecure channel is not configured for .NET Core…
Browse files Browse the repository at this point in the history
… 3.1 with gRPC (#2691)

* feat: Fail fast when unsecure channel is not configured for .NET Core 3.1

Signed-off-by: Tom Kerkhove <[email protected]>

* Update changelog

Signed-off-by: Tom Kerkhove <[email protected]>

* NotSupportedException -> InvalidOperationException

Signed-off-by: Tom Kerkhove <[email protected]>

* Code quality

Signed-off-by: Tom Kerkhove <[email protected]>

* Use static class, instead of base class

Signed-off-by: Tom Kerkhove <[email protected]>

* Revert change

Signed-off-by: Tom Kerkhove <[email protected]>

* Add missing file header

Signed-off-by: Tom Kerkhove <[email protected]>

* Change build directive

Signed-off-by: Tom Kerkhove <[email protected]>

* Code linting

Signed-off-by: Tom Kerkhove <[email protected]>

* Remove redundant check

Signed-off-by: Tom Kerkhove <[email protected]>

* Update changelog

Signed-off-by: Tom Kerkhove <[email protected]>

* Fix build errors after rebase

Signed-off-by: Tom Kerkhove <[email protected]>

* Use previous build directives

Signed-off-by: Tom Kerkhove <[email protected]>

* Update changelog with latest RC

Signed-off-by: Tom Kerkhove <[email protected]>

* Check for version, instead of build directive

Signed-off-by: Tom Kerkhove <[email protected]>

* Align tests

Signed-off-by: Tom Kerkhove <[email protected]>

* Change error message

Co-authored-by: Alan West <[email protected]>

* Change comment

Co-authored-by: Alan West <[email protected]>

* Change test name

Co-authored-by: Alan West <[email protected]>

* Change test name

Co-authored-by: Alan West <[email protected]>

* fix: Build integration test image with SDK of specified .NET version

Signed-off-by: Tom Kerkhove <[email protected]>

* revert: Build in 6.0 due to language version

Signed-off-by: Tom Kerkhove <[email protected]>

* Use version check instead of build directive

Signed-off-by: Tom Kerkhove <[email protected]>

* Fix formatting

Signed-off-by: Tom Kerkhove <[email protected]>

* Fix check on version

Signed-off-by: Tom Kerkhove <[email protected]>

* Remove flaky test

* Incorporate @pellared's feedback

Signed-off-by: Tom Kerkhove <[email protected]>

* Change code style

Signed-off-by: Tom Kerkhove <[email protected]>

* Code review remarks

Signed-off-by: Tom Kerkhove <[email protected]>

* Update changelog

Co-authored-by: Cijo Thomas <[email protected]>

* Update changelog

Co-authored-by: Cijo Thomas <[email protected]>

Co-authored-by: Alan West <[email protected]>
Co-authored-by: Cijo Thomas <[email protected]>
  • Loading branch information
3 people authored Dec 17, 2021
1 parent fd2aa0b commit 1191a2a
Show file tree
Hide file tree
Showing 9 changed files with 246 additions and 3 deletions.
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Added validation that insecure channel is configured correctly when using
.NET Core 3.x for gRPC-based exporting.
([#2691](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2691))

## 1.2.0-rc1

Released 2021-Nov-29
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
using System.Threading;
using System.Threading.Tasks;
using Grpc.Core;
using OpenTelemetry.Internal;
#if NETSTANDARD2_1 || NET5_0_OR_GREATER
using Grpc.Net.Client;
#endif
using OpenTelemetry.Internal;

namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient
{
Expand All @@ -33,6 +33,8 @@ protected BaseOtlpGrpcExportClient(OtlpExporterOptions options)
Guard.Null(options, nameof(options));
Guard.InvalidTimeout(options.TimeoutMilliseconds, nameof(options.TimeoutMilliseconds));

ExporterClientValidation.EnsureUnencryptedSupportIsEnabled(options);

this.Options = options;
this.Headers = options.GetMetadataFromHeaders();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// <copyright file="ExporterClientValidation.cs" company="OpenTelemetry Authors">
// Copyright The 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;

namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient
{
internal static class ExporterClientValidation
{
internal static void EnsureUnencryptedSupportIsEnabled(OtlpExporterOptions options)
{
var version = System.Environment.Version;

// This verification is only required for .NET Core 3.x
if (version.Major != 3)
{
return;
}

if (options.Endpoint.Scheme.Equals("http", StringComparison.InvariantCultureIgnoreCase))
{
if (AppContext.TryGetSwitch(
"System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", out var unencryptedIsSupported) == false
|| unencryptedIsSupported == false)
{
throw new InvalidOperationException(
"Calling insecure gRPC services on .NET Core 3.x requires enabling the 'System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport' switch. See: https://docs.microsoft.com/aspnet/core/grpc/troubleshoot#call-insecure-grpc-services-with-net-core-client");
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// <copyright file="ExporterClientValidationTests.cs" company="OpenTelemetry Authors">
// Copyright The 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;
using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient;
using Xunit;

namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests
{
public class ExporterClientValidationTests : Http2UnencryptedSupportTests
{
private const string HttpEndpoint = "http://localhost:4173";
private const string HttpsEndpoint = "https://localhost:4173";

[Fact]
public void ExporterClientValidation_FlagIsEnabledForHttpEndpoint()
{
var options = new OtlpExporterOptions
{
Endpoint = new Uri(HttpEndpoint),
};

AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true);

var exception = Record.Exception(() => ExporterClientValidation.EnsureUnencryptedSupportIsEnabled(options));
Assert.Null(exception);
}

[Fact]
public void ExporterClientValidation_FlagIsNotEnabledForHttpEndpoint()
{
var options = new OtlpExporterOptions
{
Endpoint = new Uri(HttpEndpoint),
};

AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", false);

var exception = Record.Exception(() => ExporterClientValidation.EnsureUnencryptedSupportIsEnabled(options));

if (Environment.Version.Major == 3)
{
Assert.NotNull(exception);
Assert.IsType<InvalidOperationException>(exception);
}
else
{
Assert.Null(exception);
}
}

[Fact]
public void ExporterClientValidation_FlagIsNotEnabledForHttpsEndpoint()
{
var options = new OtlpExporterOptions
{
Endpoint = new Uri(HttpsEndpoint),
};

var exception = Record.Exception(() => ExporterClientValidation.EnsureUnencryptedSupportIsEnabled(options));
Assert.Null(exception);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// <copyright file="Http2UnencryptedSupportTests.cs" company="OpenTelemetry Authors">
// Copyright The 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;

namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests
{
public class Http2UnencryptedSupportTests : IDisposable
{
private readonly bool initialFlagStatus;

public Http2UnencryptedSupportTests()
{
this.initialFlagStatus = this.DetermineInitialFlagStatus();
}

public void Dispose()
{
AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", this.initialFlagStatus);
}

private bool DetermineInitialFlagStatus()
{
if (AppContext.TryGetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", out var flag))
{
return flag;
}

return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,32 @@ public void ExportResultIsSuccess(OtlpExportProtocol protocol, string endpoint)
Assert.Single(delegatingExporter.ExportResults);
Assert.Equal(ExportResult.Success, delegatingExporter.ExportResults[0]);
}

[Trait("CategoryName", "CollectorIntegrationTests")]
[SkipUnlessEnvVarFoundFact(CollectorHostnameEnvVarName)]
public void ConstructingGrpcExporterFailsWhenHttp2UnencryptedSupportIsDisabledForNetcoreapp31()
{
// Adding the OtlpExporter creates a GrpcChannel.
// This switch must be set before creating a GrpcChannel/HttpClient when calling an insecure gRPC service.
// We want to fail fast so we are disabling it
// See: https://docs.microsoft.com/aspnet/core/grpc/troubleshoot#call-insecure-grpc-services-with-net-core-client
AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", false);

var exporterOptions = new OtlpExporterOptions
{
Endpoint = new Uri($"http://{CollectorHostname}:4317"),
};

var exception = Record.Exception(() => new OtlpTraceExporter(exporterOptions));

if (Environment.Version.Major == 3)
{
Assert.NotNull(exception);
}
else
{
Assert.Null(exception);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\DelegatingTestExporter.cs" Link="Includes\DelegatingTestExporter.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\EventSourceTestHelper.cs" Link="Includes\EventSourceTestHelper.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\SkipUnlessEnvVarFoundTheoryAttribute.cs" Link="Includes\SkipUnlessEnvVarFoundTheoryAttribute.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\SkipUnlessEnvVarFoundFactAttribute.cs" Link="Includes\SkipUnlessEnvVarFoundFactAttribute.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\TestActivityProcessor.cs" Link="Includes\TestActivityProcessor.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\TestEventListener.cs" Link="Includes\TestEventListener.cs" />
<Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\Utils.cs" Link="Includes\Utils.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests
{
public class OtlpExporterOptionsExtensionsTests
public class OtlpExporterOptionsExtensionsTests : Http2UnencryptedSupportTests
{
[Theory]
[InlineData("key=value", new string[] { "key" }, new string[] { "value" })]
Expand Down Expand Up @@ -86,6 +86,14 @@ public void GetHeaders_NoOptionHeaders_ReturnsEmptyHeadres(string optionHeaders)
[InlineData(OtlpExportProtocol.HttpProtobuf, typeof(OtlpHttpTraceExportClient))]
public void GetTraceExportClient_SupportedProtocol_ReturnsCorrectExportClient(OtlpExportProtocol protocol, Type expectedExportClientType)
{
if (protocol == OtlpExportProtocol.Grpc && Environment.Version.Major == 3)
{
// Adding the OtlpExporter creates a GrpcChannel.
// This switch must be set before creating a GrpcChannel when calling an insecure HTTP/2 endpoint.
// See: https://docs.microsoft.com/aspnet/core/grpc/troubleshoot#call-insecure-grpc-services-with-net-core-client
AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true);
}

var options = new OtlpExporterOptions
{
Protocol = protocol,
Expand All @@ -96,6 +104,32 @@ public void GetTraceExportClient_SupportedProtocol_ReturnsCorrectExportClient(Ot
Assert.Equal(expectedExportClientType, exportClient.GetType());
}

[Fact]
public void GetTraceExportClient_GetClientForGrpcWithoutUnencryptedFlag_ThrowsException()
{
// Adding the OtlpExporter creates a GrpcChannel.
// This switch must be set before creating a GrpcChannel when calling an insecure HTTP/2 endpoint.
// See: https://docs.microsoft.com/aspnet/core/grpc/troubleshoot#call-insecure-grpc-services-with-net-core-client
AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", false);

var options = new OtlpExporterOptions
{
Protocol = OtlpExportProtocol.Grpc,
};

var exception = Record.Exception(() => options.GetTraceExportClient());

if (Environment.Version.Major == 3)
{
Assert.NotNull(exception);
Assert.IsType<InvalidOperationException>(exception);
}
else
{
Assert.Null(exception);
}
}

[Fact]
public void GetTraceExportClient_UnsupportedProtocol_Throws()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests
{
public class OtlpTraceExporterTests
public class OtlpTraceExporterTests : Http2UnencryptedSupportTests
{
static OtlpTraceExporterTests()
{
Expand Down Expand Up @@ -372,6 +372,14 @@ public void ToOtlpSpanPeerServiceTest()
[Fact]
public void UseOpenTelemetryProtocolActivityExporterWithCustomActivityProcessor()
{
if (Environment.Version.Major == 3)
{
// Adding the OtlpExporter creates a GrpcChannel.
// This switch must be set before creating a GrpcChannel when calling an insecure HTTP/2 endpoint.
// See: https://docs.microsoft.com/aspnet/core/grpc/troubleshoot#call-insecure-grpc-services-with-net-core-client
AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true);
}

const string ActivitySourceName = "otlp.test";
TestActivityProcessor testActivityProcessor = new TestActivityProcessor();

Expand Down

0 comments on commit 1191a2a

Please sign in to comment.