-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Azure Monitor Exporter - Add Connection String #14621
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
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
a40781b
implementation
24f0db0
tests
c707acf
tests are passing
b539e98
cleanup
0619347
cleanup and more tests
fca4d58
missing test
e69e8d7
rename
a34ba37
more tests!
5a567d5
will
e1009c5
update message
35344bd
addressing code review comments
2c5379f
change exception thrown.
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
127 changes: 127 additions & 0 deletions
127
...onitor/OpenTelemetry.Exporter.AzureMonitor/src/ConnectionString/ConnectionStringParser.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using System; | ||
| using System.Linq; | ||
|
|
||
| namespace OpenTelemetry.Exporter.AzureMonitor.ConnectionString | ||
| { | ||
| internal static class ConnectionStringParser | ||
| { | ||
| /// <summary> | ||
| /// Parse a connection string that matches the format: "key1=value1;key2=value2;key3=value3". | ||
| /// This method will encapsulate all exception handling. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Official Doc: <a href="https://docs.microsoft.com/azure/azure-monitor/app/sdk-connection-string" />. | ||
| /// </remarks> | ||
| /// <exception cref="InvalidOperationException"> | ||
| /// Any exceptions that occur while parsing the connection string will be wrapped and re-thrown. | ||
| /// </exception> | ||
| public static void GetValues(string connectionString, out string instrumentationKey, out string ingestionEndpoint) | ||
| { | ||
| try | ||
| { | ||
| if (connectionString == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(connectionString)); | ||
| } | ||
| else if (connectionString.Length > Constants.ConnectionStringMaxLength) | ||
| { | ||
| throw new ArgumentOutOfRangeException(nameof(connectionString), $"Values greater than {Constants.ConnectionStringMaxLength} characters are not allowed."); | ||
| } | ||
|
|
||
| var connString = Azure.Core.ConnectionString.Parse(connectionString); | ||
| instrumentationKey = connString.GetInstrumentationKey(); | ||
| ingestionEndpoint = connString.GetIngestionEndpoint(); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| AzureMonitorTraceExporterEventSource.Log.ConnectionStringError(ex); | ||
| throw new InvalidOperationException("Connection String Error: " + ex.Message, ex); | ||
| } | ||
| } | ||
|
|
||
| internal static string GetInstrumentationKey(this Azure.Core.ConnectionString connectionString) => connectionString.GetRequired(Constants.InstrumentationKeyKey); | ||
|
|
||
| /// <summary> | ||
| /// Evaluate connection string and return the requested endpoint. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Parsing the connection string MUST follow these rules: | ||
| /// 1. check for explicit endpoint (location is ignored) | ||
| /// 2. check for endpoint suffix (location is optional) | ||
| /// 3. use default endpoint (location is ignored) | ||
| /// This behavior is required by the Connection String Specification. | ||
| /// </remarks> | ||
| internal static string GetIngestionEndpoint(this Azure.Core.ConnectionString connectionString) | ||
| { | ||
| // Passing the user input values through the Uri constructor will verify that we've built a valid endpoint. | ||
| Uri uri; | ||
|
|
||
| if (connectionString.TryGetNonRequiredValue(Constants.IngestionExplicitEndpointKey, out string explicitEndpoint)) | ||
| { | ||
| if (!Uri.TryCreate(explicitEndpoint, UriKind.Absolute, out uri)) | ||
| { | ||
| throw new ArgumentException($"The value for {Constants.IngestionExplicitEndpointKey} is invalid. '{explicitEndpoint}'"); | ||
| } | ||
| } | ||
| else if (connectionString.TryGetNonRequiredValue(Constants.EndpointSuffixKey, out string endpointSuffix)) | ||
| { | ||
| var location = connectionString.GetNonRequired(Constants.LocationKey); | ||
| if (!TryBuildUri(prefix: Constants.IngestionPrefix, suffix: endpointSuffix, location: location, uri: out uri)) | ||
| { | ||
| throw new ArgumentException($"The value for {Constants.EndpointSuffixKey} is invalid. '{endpointSuffix}'"); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| return Constants.DefaultIngestionEndpoint; | ||
| } | ||
|
|
||
| return uri.AbsoluteUri; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Construct a Uri from the possible parts. | ||
| /// Format: "location.prefix.suffix". | ||
| /// Example: "https://westus2.dc.applicationinsights.azure.cn/". | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Will also attempt to sanitize user input. Won't fail if the user typo-ed an extra period. | ||
| /// </remarks> | ||
| internal static bool TryBuildUri(string prefix, string suffix, out Uri uri, string location = null) | ||
| { | ||
| // Location and Suffix are user input fields and need to be sanitized (extra spaces or periods). | ||
| char[] trimPeriod = new char[] { '.' }; | ||
|
|
||
| if (location != null) | ||
| { | ||
| location = location.Trim().TrimEnd(trimPeriod); | ||
|
|
||
| // Location names are expected to match Azure region names. No special characters allowed. | ||
| if (!location.All(x => char.IsLetterOrDigit(x))) | ||
| { | ||
| throw new ArgumentException($"The value for Location must contain only alphanumeric characters. '{location}'"); | ||
| } | ||
| } | ||
|
|
||
| var uriString = string.Concat("https://", | ||
| string.IsNullOrEmpty(location) ? string.Empty : (location + "."), | ||
TimothyMothra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| prefix, | ||
| ".", | ||
| suffix.Trim().TrimStart(trimPeriod)); | ||
|
|
||
| return Uri.TryCreate(uriString, UriKind.Absolute, out uri); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// This method wraps <see cref="Azure.Core.ConnectionString.GetNonRequired(string)"/> in a null check. | ||
| /// </summary> | ||
| internal static bool TryGetNonRequiredValue(this Azure.Core.ConnectionString connectionString, string key, out string value) | ||
| { | ||
| value = connectionString.GetNonRequired(key); | ||
| return value != null; | ||
| } | ||
| } | ||
| } | ||
49 changes: 49 additions & 0 deletions
49
sdk/monitor/OpenTelemetry.Exporter.AzureMonitor/src/ConnectionString/Constants.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| namespace OpenTelemetry.Exporter.AzureMonitor.ConnectionString | ||
| { | ||
| internal static class Constants | ||
| { | ||
| /// <summary> | ||
| /// Default endpoint for Ingestion (aka Breeze). | ||
| /// </summary> | ||
| internal const string DefaultIngestionEndpoint = "https://dc.services.visualstudio.com/"; | ||
|
|
||
| /// <summary> | ||
| /// Sub-domain for Ingestion endpoint (aka Breeze). (https://dc.applicationinsights.azure.com/). | ||
| /// </summary> | ||
| internal const string IngestionPrefix = "dc"; | ||
|
|
||
| /// <summary> | ||
| /// This is the key that a customer would use to specify an explicit endpoint in the connection string. | ||
| /// </summary> | ||
| internal const string IngestionExplicitEndpointKey = "IngestionEndpoint"; | ||
|
|
||
| /// <summary> | ||
| /// This is the key that a customer would use to specify an instrumentation key in the connection string. | ||
| /// </summary> | ||
| internal const string InstrumentationKeyKey = "InstrumentationKey"; | ||
|
|
||
| /// <summary> | ||
| /// This is the key that a customer would use to specify an endpoint suffix in the connection string. | ||
| /// </summary> | ||
| internal const string EndpointSuffixKey = "EndpointSuffix"; | ||
|
|
||
| /// <summary> | ||
| /// This is the key that a customer would use to specify a location in the connection string. | ||
| /// </summary> | ||
| internal const string LocationKey = "Location"; | ||
|
|
||
| /// <summary> | ||
| /// Maximum allowed length for connection string. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Currently 8 accepted keywords (~200 characters). | ||
| /// Assuming 200 characters per value (~1600 characters). | ||
| /// Total theoretical max length: (1600 + 200) = 1800. | ||
| /// Setting an over-exaggerated max length to protect against malicious injections (2^12 = 4096). | ||
| /// </remarks> | ||
| internal const int ConnectionStringMaxLength = 4096; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
77 changes: 77 additions & 0 deletions
77
...itor/tests/OpenTelemetry.Exporter.AzureMonitor.UnitTest/AzureMonitorTraceExporterTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using System; | ||
| using System.Reflection; | ||
|
|
||
| using NUnit.Framework; | ||
|
|
||
| namespace OpenTelemetry.Exporter.AzureMonitor | ||
| { | ||
| public class AzureMonitorTraceExporterTests | ||
| { | ||
| [Test] | ||
| public void VerifyConnectionString_CorrectlySetsEndpoint() | ||
| { | ||
| var testIkey = "test_ikey"; | ||
| var testEndpoint = "https://www.bing.com/"; | ||
|
|
||
| var exporter = new AzureMonitorTraceExporter(new AzureMonitorExporterOptions { ConnectionString = $"InstrumentationKey={testIkey};IngestionEndpoint={testEndpoint}" }); | ||
|
|
||
| GetInternalFields(exporter, out string ikey, out string endpoint); | ||
| Assert.AreEqual(testIkey, ikey); | ||
| Assert.AreEqual(testEndpoint, endpoint); | ||
| } | ||
|
|
||
| [Test] | ||
| public void VerifyConnectionString_CorrectlySetsDefaultEndpoint() | ||
| { | ||
| var testIkey = "test_ikey"; | ||
|
|
||
| var exporter = new AzureMonitorTraceExporter(new AzureMonitorExporterOptions { ConnectionString = $"InstrumentationKey={testIkey};" }); | ||
|
|
||
| GetInternalFields(exporter, out string ikey, out string endpoint); | ||
| Assert.AreEqual(testIkey, ikey); | ||
| Assert.AreEqual(ConnectionString.Constants.DefaultIngestionEndpoint, endpoint); | ||
| } | ||
|
|
||
| [Test] | ||
| public void VerifyConnectionString_ThrowsExceptionWhenInvalid() | ||
| { | ||
| Assert.Throws<InvalidOperationException>(() => new AzureMonitorTraceExporter(new AzureMonitorExporterOptions { ConnectionString = null })); | ||
| } | ||
|
|
||
| [Test] | ||
| public void VerifyConnectionString_ThrowsExceptionWhenMissingInstrumentationKey() | ||
| { | ||
| var testEndpoint = "https://www.bing.com/"; | ||
|
|
||
| Assert.Throws<InvalidOperationException>(() => new AzureMonitorTraceExporter(new AzureMonitorExporterOptions { ConnectionString = $"IngestionEndpoint={testEndpoint}" })); | ||
| } | ||
|
|
||
| private void GetInternalFields(AzureMonitorTraceExporter exporter, out string ikey, out string endpoint) | ||
| { | ||
| // TODO: NEED A BETTER APPROACH FOR TESTING. WE DECIDED AGAINST MAKING FIELDS "internal". | ||
| // instrumentationKey: AzureMonitorTraceExporter.AzureMonitorTransmitter.instrumentationKey | ||
| // endpoint: AzureMonitorTraceExporter.AzureMonitorTransmitter.ServiceRestClient.endpoint | ||
|
|
||
| var transmitter = typeof(AzureMonitorTraceExporter) | ||
| .GetField("AzureMonitorTransmitter", BindingFlags.Instance | BindingFlags.NonPublic) | ||
| .GetValue(exporter); | ||
|
|
||
| ikey = typeof(AzureMonitorTransmitter) | ||
| .GetField("instrumentationKey", BindingFlags.Instance | BindingFlags.NonPublic) | ||
| .GetValue(transmitter) | ||
| .ToString(); | ||
|
|
||
| var serviceRestClient = typeof(AzureMonitorTransmitter) | ||
| .GetField("serviceRestClient", BindingFlags.Instance | BindingFlags.NonPublic) | ||
| .GetValue(transmitter); | ||
|
|
||
| endpoint = typeof(ServiceRestClient) | ||
| .GetField("endpoint", BindingFlags.Instance | BindingFlags.NonPublic) | ||
| .GetValue(serviceRestClient) | ||
| .ToString(); | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.