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.

Sign up for GitHub

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

[Exporter.Geneva] Move TableNameMappings checks to GenevaExporterOptions class #646

Merged
merged 4 commits into from
Sep 17, 2022
Merged

[Exporter.Geneva] Move TableNameMappings checks to GenevaExporterOptions class #646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c0146c
Move the checks from ctor exporters to the options class
utpilla Sep 16, 2022
17951a8
Merge remote-tracking branch 'origin/main' into utpilla/Update-Geneva…
utpilla Sep 16, 2022
0c647f1
Update CHANGELOG
utpilla Sep 17, 2022
224ae61
Remove unnecessary using statements
utpilla Sep 17, 2022
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
7 changes: 6 additions & 1 deletion src/OpenTelemetry.Exporter.Geneva/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
## Unreleased

* Update OTel SDK version to `1.3.1`.
([#631](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/631))
[#631](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/631)

* The option `TableNameMappings` of `GenevaExporterOptions` will not support
string values that are null, empty, or consist only of white-space characters.
It will also not support string values that contain non-ASCII characters.
[646](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/646)

## 1.4.0-beta.1

Expand Down
10 changes: 8 additions & 2 deletions src/OpenTelemetry.Exporter.Geneva/GenevaExporterOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

using System;
using System.Collections.Generic;
using System.Text;
using OpenTelemetry.Internal;

namespace OpenTelemetry.Exporter.Geneva;
Expand Down Expand Up @@ -44,9 +45,14 @@ public IReadOnlyDictionary<string, string> TableNameMappings

foreach (var entry in value)
{
if (entry.Value is null)
if (string.IsNullOrWhiteSpace(entry.Value))
{
throw new ArgumentNullException(entry.Key, $"{nameof(this.TableNameMappings)} must not contain null values.");
throw new ArgumentException($"A string-typed value provided for {nameof(this.TableNameMappings)} must not be null, empty, or consist only of white-space characters.");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check didn't exist for GenevaLogExporter.

}

if (Encoding.UTF8.GetByteCount(entry.Value) != entry.Value.Length)
{
throw new ArgumentException($"A string-typed value provided for {nameof(this.TableNameMappings)} must not contain non-ASCII characters.", entry.Value);
}

copy[entry.Key] = entry.Value;
Expand Down
6 changes: 0 additions & 6 deletions src/OpenTelemetry.Exporter.Geneva/GenevaLogExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;
using Microsoft.Extensions.Logging;
using OpenTelemetry.Internal;
Expand Down Expand Up @@ -58,11 +57,6 @@ public GenevaLogExporter(GenevaExporterOptions options)
var tempTableMappings = new Dictionary<string, string>(options.TableNameMappings.Count, StringComparer.Ordinal);
foreach (var kv in options.TableNameMappings)
{
if (Encoding.UTF8.GetByteCount(kv.Value) != kv.Value.Length)
{
throw new ArgumentException("The value: \"{tableName}\" provided for TableNameMappings option contains non-ASCII characters", kv.Value);
}

if (kv.Key == "*")
{
if (kv.Value == "*")
Expand Down
11 changes: 0 additions & 11 deletions src/OpenTelemetry.Exporter.Geneva/GenevaTraceExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
using System.Diagnostics;
using System.Linq;
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;
using OpenTelemetry.Internal;

Expand All @@ -36,16 +35,6 @@ public GenevaTraceExporter(GenevaExporterOptions options)
if (options.TableNameMappings != null
&& options.TableNameMappings.TryGetValue("Span", out var customTableName))
{
if (string.IsNullOrWhiteSpace(customTableName))
{
throw new ArgumentException("TableName mapping for Span is invalid.");
}

if (Encoding.UTF8.GetByteCount(customTableName) != customTableName.Length)
{
throw new ArgumentException("The \"{customTableName}\" provided for TableNameMappings option contains non-ASCII characters", customTableName);
}

partAName = customTableName;
}

Expand Down
5 changes: 2 additions & 3 deletions test/OpenTelemetry.Exporter.Geneva.Tests/GenevaLogExporterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,14 @@ public void SpecialCharactersInTableNameMappings()
});

// Throw on null value - include key in exception message
var ex = Assert.Throws<ArgumentNullException>(() =>
var ex = Assert.Throws<ArgumentException>(() =>
{
new GenevaExporterOptions
{
TableNameMappings = new Dictionary<string, string> { ["TestCategory"] = null },
};
});
Assert.Contains("TableNameMappings must not contain null values.", ex.Message);
Assert.Equal("TestCategory", ex.ParamName);
Assert.Contains("A string-typed value provided for TableNameMappings must not be null, empty, or consist only of white-space characters.", ex.Message);

// Throw when TableNameMappings is null
Assert.Throws<ArgumentNullException>(() =>
Expand Down