Skip to content

[Azure.Monitor] Add sovereign support for Query and Ingestion #37819

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

Closed
nisha-bhatia wants to merge 8 commits into from
Closed

[Azure.Monitor] Add sovereign support for Query and Ingestion #37819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b78da57
Update LogsQueryClient.cs
nisha-bhatia Jul 18, 2023
cb30d24
wip
nisha-bhatia Jul 24, 2023
c43645e
wip
nisha-bhatia Jul 24, 2023
ce69668
wip
nisha-bhatia Jul 25, 2023
49e4bfa
wip
nisha-bhatia Jul 28, 2023
1b97ca1
wip
nisha-bhatia Aug 23, 2023
a0ef46c
Update MonitorQueryTestEnvironment.cs
nisha-bhatia Aug 23, 2023
8a982db
Update MonitorQueryTestEnvironment.cs
nisha-bhatia Aug 23, 2023
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
1 change: 1 addition & 0 deletions sdk/monitor/Azure.Monitor.Query/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
### Bugs Fixed

### Other Changes
- Added testing for sovereign support for `LogsQueryClient` and `MetricsQueryClient`

## 1.2.0 (2023-05-22)

Expand Down
1 change: 0 additions & 1 deletion sdk/monitor/Azure.Monitor.Query/src/LogsQueryClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Text;
using System.Text.Json;
using System.Threading;
Expand Down
3 changes: 2 additions & 1 deletion sdk/monitor/Azure.Monitor.Query/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ extends:
parameters:
ServiceDirectory: monitor
Project: Azure.Monitor.Query
TimeoutInMinutes: 120
TimeoutInMinutes: 240
SupportedClouds: 'Public,UsGov,China'
29 changes: 28 additions & 1 deletion sdk/monitor/Azure.Monitor.Query/tests/MonitorQueryTestEnvironment.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using Azure.Core.TestFramework;
using NUnit.Framework;

namespace Azure.Monitor.Query.Tests
{
Expand All @@ -16,10 +18,35 @@ public class MonitorQueryTestEnvironment : TestEnvironment
public string MetricsNamespace => GetRecordedVariable("METRICS_RESOURCE_NAMESPACE");
public string MonitorIngestionEndpoint => GetOptionalVariable("METRICS_INGEST_SUFFIX") ?? "ods.opinsights.azure.com";
public string MetricsIngestionEndpoint => GetOptionalVariable("METRICS_INGEST_SUFFIX") ?? "monitoring.azure.com";
public Uri LogsEndpoint => new(GetRecordedVariable("LOGS_ENDPOINT"));
public Uri MetricsEndpoint => new(ResourceManagerUrl);
public string ResourceId => GetRecordedVariable("RESOURCE_ID");
public string WorkspacePrimaryResourceId => GetRecordedVariable("WORKSPACE_PRIMARY_RESOURCE_ID");
public string WorkspaceSecondaryResourceId => GetRecordedVariable("WORKSPACE_SECONDARY_RESOURCE_ID");
public Uri LogsEndpoint => new(GetEndpoint());
Copy link
Copy Markdown
Member

@JoshLove-msft JoshLove-msft Jul 25, 2023

Choose a reason for hiding this comment

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

This will fail during playback as we are no longer looking up the recorded variable.

Copy link
Copy Markdown
Contributor Author

@nisha-bhatia nisha-bhatia Jul 25, 2023

Choose a reason for hiding this comment

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

I can check if the mode is playback and set it to the default?

Copy link
Copy Markdown
Member

@JoshLove-msft JoshLove-msft Jul 25, 2023

Choose a reason for hiding this comment

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

No, if it is playback it would need to check the recorded value in the session record, which is what GetRecordedVariable does.

It would probably be cleaner if we could move the GetEndpoint logic directly into the bicep file so that we are setting the LOGS_ENDPOINT env var based on the Azure Environment.

Copy link
Copy Markdown
Member

@JoshLove-msft JoshLove-msft Jul 25, 2023

Choose a reason for hiding this comment

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

If we don't want to go that route, then we would at least need to condition on whether we are in PlayBack mode and if so, call GetRecordedVariable rather than GetEndpoint.


private Dictionary<string, string> regions = new Dictionary<string, string>()
{
{ "AzureCloud", "https://api.loganalytics.io/v1" },
Copy link
Copy Markdown
Member

@JoshLove-msft JoshLove-msft Jul 25, 2023

Choose a reason for hiding this comment

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

Is it not possible to pull this down via the bicep file?

Copy link
Copy Markdown
Contributor Author

@nisha-bhatia nisha-bhatia Jul 25, 2023

Choose a reason for hiding this comment

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

No I don't believe so. Other languages are pulling from the test environment as well

Copy link
Copy Markdown
Member

@JoshLove-msft JoshLove-msft Jul 25, 2023

Choose a reason for hiding this comment

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

So the service doesn't expose these endpoints programmatically? Users have to look at docs?

Copy link
Copy Markdown
Contributor

@pallavit pallavit Jul 25, 2023

Choose a reason for hiding this comment

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

Not sure about monitor in particular but a lot of services have this hard coded and expect people to discover this through documentation.

Copy link
Copy Markdown
Contributor

@pallavit pallavit Jul 25, 2023

Choose a reason for hiding this comment

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

I am curious about v1 in the end - Does the service have v2 endpoints too? Is this their versioning scheme?

Copy link
Copy Markdown
Contributor Author

@nisha-bhatia nisha-bhatia Jul 25, 2023

Choose a reason for hiding this comment

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

Yes the service is in v1 right now and v2 I'm assuming will come out in the future.

{ "AzureChinaCloud", "https://api.loganalytics.azure.cn/v1" },
{ "AzureUSGovernment", "https://api.loganalytics.us/v1" }
};

private string ENV_MONITOR_ENVIRONMENT = "MONITOR_ENVIRONMENT";
private string GetEndpoint()
{
// if mode is Playback use DefaultEndpoint
if (Mode == RecordedTestMode.Playback)
{
return GetRecordedVariable("LOGS_ENDPOINT");
}
// else find endpoing of specific region from pipeline
string endpoint = "";
TestContext.Progress.WriteLine("Current Region: " + Environment.GetEnvironmentVariable(ENV_MONITOR_ENVIRONMENT));
if (regions.TryGetValue(Environment.GetEnvironmentVariable(ENV_MONITOR_ENVIRONMENT), out string region))
Copy link
Copy Markdown
Member

@JoshLove-msft JoshLove-msft Jul 25, 2023

Choose a reason for hiding this comment

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

How does this environment variable get set?

Copy link
Copy Markdown
Member

@JoshLove-msft JoshLove-msft Jul 25, 2023

Choose a reason for hiding this comment

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

Ah it is here - https://github.com/Azure/azure-sdk-for-net/blob/main/eng/common/TestResources/New-TestResources.ps1#L273

Copy link
Copy Markdown
Contributor Author

@nisha-bhatia nisha-bhatia Jul 25, 2023

Choose a reason for hiding this comment

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

thank you!

{
endpoint = region;
}
return endpoint;
}
}
}
2 changes: 1 addition & 1 deletion sdk/monitor/test-resources.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ output WORKSPACE_KEY string = listKeys(primaryWorkspace.id, '2020-10-01').primar
output SECONDARY_WORKSPACE_KEY string = listKeys(secondaryWorkspace.id, '2020-10-01').primarySharedKey
output METRICS_RESOURCE_ID string = primaryWorkspace.id
output METRICS_RESOURCE_NAMESPACE string = 'Microsoft.OperationalInsights/workspaces'
output LOGS_ENDPOINT string = 'https://api.loganalytics.io'
output MONITOR_INGESTION_DATA_COLLECTION_ENDPOINT string = dataCollectionEndpoint.properties.logsIngestion.endpoint
output LOGS_ENDPOINT string = 'https://api.loganalytics.io'
output INGESTION_STREAM_NAME string = streamName
output INGESTION_TABLE_NAME string = table.name
output INGESTION_DATA_COLLECTION_RULE_ID string = dataCollectionRule.id
Expand Down