Skip to content

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

Closed
nisha-bhatia wants to merge 8 commits intoAzure:mainfrom
nisha-bhatia:nibhatiQuerySovereign
Closed

[Azure.Monitor] Add sovereign support for Query and Ingestion#37819
nisha-bhatia wants to merge 8 commits intoAzure:mainfrom
nisha-bhatia:nibhatiQuerySovereign

Conversation

@nisha-bhatia
Copy link
Contributor

@nisha-bhatia nisha-bhatia commented Jul 24, 2023

Fixes #32169

@nisha-bhatia nisha-bhatia added Client This issue is related to a non-management package Monitor - Query labels Jul 24, 2023
@nisha-bhatia nisha-bhatia self-assigned this Jul 24, 2023
@nisha-bhatia nisha-bhatia marked this pull request as draft July 24, 2023 23:39
@nisha-bhatia
Copy link
Contributor Author

Ingestion is still a wip, will be added later

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.


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

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
Contributor Author

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
Member

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
Contributor

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
Contributor

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
Contributor Author

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.

private string GetEndpoint()
{
string endpoint = "";
if (regions.TryGetValue(Environment.GetEnvironmentVariable(ENV_MONITOR_ENVIRONMENT), out string region))
Copy link
Member

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
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you!

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
Member

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
Contributor Author

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
Member

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
Member

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.

@nisha-bhatia
Copy link
Contributor Author

/azp run net - Azure.Monitor.Query - tests-weekly

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scottaddie
Copy link
Member

@nisha-bhatia Reminder to also update the READMEs in this PR

@nisha-bhatia
Copy link
Contributor Author

/azp run net - Azure.Monitor.Query - tests-weekly

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nisha-bhatia
Copy link
Contributor Author

/azp run net - Azure.Monitor.Query - tests-weekly

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nisha-bhatia
Copy link
Contributor Author

/azp run net - Azure.Monitor.Query - tests-weekly

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nisha-bhatia
Copy link
Contributor Author

/azp run net - Azure.Monitor.Query - tests-weekly

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scottaddie scottaddie added Monitor Monitor, Monitor Ingestion, Monitor Query and removed Monitor - Query labels Sep 26, 2023
@nisha-bhatia
Copy link
Contributor Author

Closing this PR: sovereign support will be addressed after metricsBatch #38640

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client This issue is related to a non-management package Monitor Monitor, Monitor Ingestion, Monitor Query

Projects

None yet

5 participants