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

feat: Fail fast when insecure channel is not configured for .NET Core 3.1 with gRPC #2691

Merged
merged 35 commits into from
Dec 17, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
d064536
feat: Fail fast when unsecure channel is not configured for .NET Core…
tomkerkhove Nov 26, 2021
c00bbfe
Update changelog
tomkerkhove Nov 26, 2021
b5883b5
NotSupportedException -> InvalidOperationException
tomkerkhove Nov 26, 2021
1cf66f5
Code quality
tomkerkhove Nov 26, 2021
f93630f
Use static class, instead of base class
tomkerkhove Nov 26, 2021
6760ab0
Revert change
tomkerkhove Nov 26, 2021
75a163a
Add missing file header
tomkerkhove Nov 26, 2021
e452bce
Change build directive
tomkerkhove Nov 29, 2021
cece9c5
Code linting
tomkerkhove Nov 29, 2021
9fc1d86
Remove redundant check
tomkerkhove Nov 29, 2021
0979d55
Update changelog
tomkerkhove Nov 29, 2021
45c5f7f
Merge remote-tracking branch 'upstream/main' into ssl-check
tomkerkhove Nov 30, 2021
57790cf
Fix build errors after rebase
tomkerkhove Nov 30, 2021
d56cfbe
Use previous build directives
tomkerkhove Nov 30, 2021
fcb826e
Update changelog with latest RC
tomkerkhove Nov 30, 2021
9971645
Check for version, instead of build directive
tomkerkhove Dec 2, 2021
16fd4da
Align tests
tomkerkhove Dec 2, 2021
351f89e
Change error message
tomkerkhove Dec 4, 2021
4eeab06
Change comment
tomkerkhove Dec 4, 2021
dfb1099
Change test name
tomkerkhove Dec 4, 2021
9c8e44c
Change test name
tomkerkhove Dec 4, 2021
ce662f4
fix: Build integration test image with SDK of specified .NET version
tomkerkhove Dec 4, 2021
bc71cf5
revert: Build in 6.0 due to language version
tomkerkhove Dec 4, 2021
102a349
Merge branch 'main' into ssl-check
tomkerkhove Dec 4, 2021
3626c4b
Merge branch 'main' into ssl-check
cijothomas Dec 7, 2021
c139b40
Use version check instead of build directive
tomkerkhove Dec 7, 2021
d3901d4
Fix formatting
tomkerkhove Dec 7, 2021
5e74b8e
Fix check on version
tomkerkhove Dec 7, 2021
b541f3d
Remove flaky test
tomkerkhove Dec 8, 2021
edb1b58
Incorporate @pellared's feedback
tomkerkhove Dec 8, 2021
3dd01b3
Change code style
tomkerkhove Dec 8, 2021
49b39d3
Code review remarks
tomkerkhove Dec 9, 2021
8df0df8
Update changelog
tomkerkhove Dec 10, 2021
f8c3668
Update changelog
tomkerkhove Dec 10, 2021
b7446aa
Merge branch 'main' into ssl-check
alanwest Dec 16, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* Added configuration options for `MetricReaderType` to allow for configuring
the `OtlpMetricExporter` to export either manually or periodically.
([#2674](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2674))
* Added validation that unsecure channel is configured correctly when using
.NET Core 3.1 for gRPC & HTTP exporting.
([#2691](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2691))

## 1.2.0-beta2

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// <copyright file="BaseOtlpExportClient.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.Internal;

namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient
{
/// <summary>Base class for sending OTLP export requests.</summary>
internal abstract class BaseOtlpExportClient
tomkerkhove marked this conversation as resolved.
Show resolved Hide resolved
{
protected BaseOtlpExportClient(OtlpExporterOptions options)
{
Guard.Null(options, nameof(options));
Guard.InvalidTimeout(options.TimeoutMilliseconds, nameof(options.TimeoutMilliseconds));

this.Options = options;

#if NETCOREAPP3_1
EnsureUnencryptedSupportIsEnabled(options);
#endif
}

internal OtlpExporterOptions Options { get; }

private static void EnsureUnencryptedSupportIsEnabled(OtlpExporterOptions options)
tomkerkhove marked this conversation as resolved.
Show resolved Hide resolved
{
if (options.Endpoint.Scheme.Equals("http", StringComparison.InvariantCultureIgnoreCase))
{
if (AppContext.TryGetSwitch(
"System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", out var unencryptedIsSupported) == false
|| unencryptedIsSupported == false)
{
throw new NotSupportedException(
tomkerkhove marked this conversation as resolved.
Show resolved Hide resolved
"'System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport' must be enabled for using HTTP with .NET 3.1");
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,14 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClie
{
/// <summary>Base class for sending OTLP export request over gRPC.</summary>
/// <typeparam name="TRequest">Type of export request.</typeparam>
internal abstract class BaseOtlpGrpcExportClient<TRequest> : IExportClient<TRequest>
internal abstract class BaseOtlpGrpcExportClient<TRequest> : BaseOtlpExportClient, IExportClient<TRequest>
{
protected BaseOtlpGrpcExportClient(OtlpExporterOptions options)
: base(options)
{
Guard.Null(options, nameof(options));
Guard.InvalidTimeout(options.TimeoutMilliseconds, nameof(options.TimeoutMilliseconds));

this.Options = options;
this.Headers = options.GetMetadataFromHeaders();
}

internal OtlpExporterOptions Options { get; }

#if NETSTANDARD2_1 || NET5_0_OR_GREATER
internal GrpcChannel Channel { get; set; }
#else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,20 @@
using System.Collections.Generic;
using System.Net.Http;
using System.Threading;
using OpenTelemetry.Internal;

namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient
{
/// <summary>Base class for sending OTLP export request over HTTP.</summary>
/// <typeparam name="TRequest">Type of export request.</typeparam>
internal abstract class BaseOtlpHttpExportClient<TRequest> : IExportClient<TRequest>
internal abstract class BaseOtlpHttpExportClient<TRequest> : BaseOtlpExportClient, IExportClient<TRequest>
{
protected BaseOtlpHttpExportClient(OtlpExporterOptions options, HttpClient httpClient = null)
: base(options)
{
Guard.Null(options, nameof(options));
Guard.InvalidTimeout(options.TimeoutMilliseconds, $"{nameof(options)}.{nameof(options.TimeoutMilliseconds)}");

this.Options = options;
this.Headers = options.GetHeaders<Dictionary<string, string>>((d, k, v) => d.Add(k, v));
this.HttpClient = httpClient ?? new HttpClient { Timeout = TimeSpan.FromMilliseconds(this.Options.TimeoutMilliseconds) };
this.HttpClient = httpClient ?? new HttpClient { Timeout = TimeSpan.FromMilliseconds(base.Options.TimeoutMilliseconds) };
}

internal OtlpExporterOptions Options { get; }

internal HttpClient HttpClient { get; }

internal IReadOnlyDictionary<string, string> Headers { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,40 @@ public void ExportResultIsSuccess()
Assert.Single(delegatingExporter.ExportResults);
Assert.Equal(ExportResult.Success, delegatingExporter.ExportResults[0]);
}

#if NETCOREAPP3_1
[Trait("CategoryName", "CollectorIntegrationTests")]
[SkipUnlessEnvVarFoundFact(CollectorEndpointEnvVarName)]
public void ExportResultIsFailedBecauseEncryptedHttpSupportIsDisabled()
{
// 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://{CollectorEndpoint}"),
};

Assert.Throws<NotSupportedException>(() => new OtlpTraceExporter(exporterOptions));
}

[Trait("CategoryName", "CollectorIntegrationTests")]
[SkipUnlessEnvVarFoundFact(CollectorEndpointEnvVarName)]
public void ExportResultIsFailedBecauseEncryptedHttpSupportIsNotConfigured()
{
// 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 not setting it to ensure it fails
tomkerkhove marked this conversation as resolved.
Show resolved Hide resolved
var exporterOptions = new OtlpExporterOptions
{
Endpoint = new Uri($"http://{CollectorEndpoint}"),
};

Assert.Throws<NotSupportedException>(() => new OtlpTraceExporter(exporterOptions));
}
#endif
}
}