Skip to content

Conversation

@arushiarora24
Copy link
Member

Contributing to the Azure SDK

Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.

@github-actions github-actions bot added the Mgmt This issue is related to a management package. label Nov 12, 2025
@github-actions
Copy link

github-actions bot commented Nov 12, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

Azure.ResourceManager.Dynatrace

@arushiarora24 arushiarora24 marked this pull request as ready for review November 12, 2025 11:07
Copilot AI review requested due to automatic review settings November 12, 2025 11:07
Copilot finished reviewing on behalf of arushiarora24 November 12, 2025 11:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refreshes the Dynatrace SDK to support the 2024-04-24 API version, introducing new operations for monitored subscriptions management, agent installation, plan upgrades, and marketplace integration while removing deprecated operations.

Key changes:

  • Updates API version from 2021-09-01 to 2024-04-24
  • Adds MonitoredSubscriptions operations for subscription-level monitoring configuration
  • Introduces agent management, plan upgrade, and marketplace SaaS resource operations
  • Removes deprecated operations (TagRules Update, Monitors GetAccountCredentials)

Reviewed Changes

Copilot reviewed 18 out of 83 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
autorest.md Updates API spec commit reference and adds new rename mappings; removes deprecated mappings and operations
GlobalSuppressions.cs Adds naming suppressions for generated model types matching service contract
MonitoredSubscriptionCollectionTests.cs New test file for monitored subscription operations
MonitoredSubscriptionsRestOperations.cs New REST client for monitored subscription CRUD operations
MonitoredSubscriptionPropertyResource.cs New resource type for managing monitored subscription properties
TagRulesRestOperations.cs Removes Update operation and 200 status code from Delete operation
SingleSignOnRestOperations.cs Updates API version and documentation
MonitorsRestOperations.cs Replaces GetAccountCredentials with ListMonitoredResources; adds new operations for agent management, metrics status, plan upgrade, and marketplace integration
Various model files New models for subscription monitoring, agent actions, marketplace resources, metrics status, and plan upgrades
LinkableEnvironmentContent.cs Changes constructor to require parameters; adds customization file for backward compatibility
DynatraceSsoDetailsContent.cs Changes constructor to require userPrincipal parameter
DynatraceMonitorResourceMetricRules.cs Changes visibility from internal to public and adds SendingMetrics property
DynatraceMonitorPatch.cs Restructures properties using nested MonitorUpdateProperties and adds Identity support

@@ -1,5 +1,8 @@
# Release History

### 2.0.0 (2025-11-27)
Copy link
Member

@live1206 live1206 Nov 13, 2025

Choose a reason for hiding this comment

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

We don't bump major version yet, update 1.2.0-beta.1 entry below with 1.2.0 instead.
Also update the version in .csproj with this version.

Copy link
Member Author

Choose a reason for hiding this comment

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

@live1206 , I have updated change log and .csproj file per your suggestion. Please let me know if you have any concerns.

Copy link
Member

Choose a reason for hiding this comment

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

we should not have this file to by pass apicompat.
please remove this file and write some customization code to mitigate this breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

Comment on lines 8 to 9
[assembly: SuppressMessage("Naming", "AZC0030:Improper model name suffix", Justification = "Generated model name matches service contract; changing would be a breaking change.", Scope = "type", Target = "~T:Azure.ResourceManager.Dynatrace.Models.MarketplaceSaaSResourceDetailsResponse")]
[assembly: SuppressMessage("Naming", "AZC0030:Improper model name suffix", Justification = "Generated model name matches service contract; changing would be a breaking change.", Scope = "type", Target = "~T:Azure.ResourceManager.Dynatrace.Models.MetricsStatusResponse")]
Copy link
Member

Choose a reason for hiding this comment

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

we also should not have this to suppress the linters, we should add renaming rules to rename those models.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -16,6 +16,9 @@ public static partial class DynatraceExtensions
public static Azure.AsyncPageable<Azure.ResourceManager.Dynatrace.DynatraceMonitorResource> GetDynatraceMonitorsAsync(this Azure.ResourceManager.Resources.SubscriptionResource subscriptionResource, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public static Azure.ResourceManager.Dynatrace.DynatraceSingleSignOnResource GetDynatraceSingleSignOnResource(this Azure.ResourceManager.ArmClient client, Azure.Core.ResourceIdentifier id) { throw null; }
public static Azure.ResourceManager.Dynatrace.DynatraceTagRuleResource GetDynatraceTagRuleResource(this Azure.ResourceManager.ArmClient client, Azure.Core.ResourceIdentifier id) { throw null; }
public static Azure.Response<Azure.ResourceManager.Dynatrace.Models.MarketplaceSaaSResourceDetailsResult> GetMarketplaceSaaSResourceDetailsMonitor(this Azure.ResourceManager.Resources.SubscriptionResource subscriptionResource, Azure.ResourceManager.Dynatrace.Models.MarketplaceSaaSResourceDetailsContent content, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
Copy link
Member

Choose a reason for hiding this comment

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

we should add a configuration to rename this operation, now the operation name does not make any sense.

/// <param name="cancellationToken"> The cancellation token to use. </param>
// Add this custom code due to the api compatibility for operation: Monitors_GetAccountCredentials.
[EditorBrowsable(EditorBrowsableState.Never)]
public virtual async Task<Response<DynatraceAccountCredentialsInfo>> GetAccountCredentialsAsync(CancellationToken cancellationToken = default)
Copy link
Member

Choose a reason for hiding this comment

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

the reason that we have to introduce this back here is that in the new api-version, we no longer have this operation.
Therefore we introduce it back as back compat.
Is this correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is correct.

/// <returns> An async collection of <see cref="DynatraceMonitoredResourceDetails"/> that may take multiple service requests to iterate over. </returns>
// Add this custom code due to previous version didn't have request body parameter.
[EditorBrowsable(EditorBrowsableState.Never)]
public virtual AsyncPageable<DynatraceMonitoredResourceDetails> GetMonitoredResourcesAsync(CancellationToken cancellationToken = default)
Copy link
Member

Choose a reason for hiding this comment

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

I saw in the generated version of this class, we still have this method, but with an extra body parameter.
Therefore we need to make two changes here:

  1. we need to make the cancellationToken parameter required here like this:
Suggested change
public virtual AsyncPageable<DynatraceMonitoredResourceDetails> GetMonitoredResourcesAsync(CancellationToken cancellationToken = default)
public virtual AsyncPageable<DynatraceMonitoredResourceDetails> GetMonitoredResourcesAsync(CancellationToken cancellationToken)
  1. we need to remove all the implementation here and call the generated overload like this:
public virtual AsyncPageable<DynatraceMonitoredResourceDetails> GetMonitoredResourcesAsync(CancellationToken cancellationToken)
=> GetMonitoredResourcesAsync(null, cancellationToken);

same change should be applied to the sync version of this method as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -6,14 +6,14 @@
using System;
using System.Collections.Generic;
using Azure.Core;
using Azure.ResourceManager.Dynatrace.Models;
// using Azure.ResourceManager.Dynatrace.Models;
Copy link
Member

Choose a reason for hiding this comment

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

please clean up the using here

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

{
/// <summary> List of filtering tags to be used for capturing metrics. If empty, all resources will be captured. If only Exclude action is specified, the rules will apply to the list of all available resources. If Include actions are specified, the rules will only include resources with the associated tags. </summary>
// Add this property due to the previous swagger definition for MetricRules only had FilteringTags as a direct child property.
public IList<DynatraceMonitorResourceFilteringTag> MetricRulesFilteringTags
Copy link
Member

Choose a reason for hiding this comment

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

Please add a [EditorBrowsable] attribute here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

Comment on lines +14 to +23
/// <summary> User info. </summary>
public DynatraceMonitorUserInfo UserInfo { get; set; }
/// <summary> Billing plan information. </summary>
public DynatraceBillingPlanInfo PlanData { get; set; }
/// <summary> Status of the monitor. </summary>
public DynatraceMonitoringStatus? MonitoringStatus { get; set; }
/// <summary> Marketplace subscription status. </summary>
public DynatraceMonitorMarketplaceSubscriptionStatus? MarketplaceSubscriptionStatus { get; set; }
/// <summary> Properties of the Dynatrace environment. </summary>
public DynatraceEnvironmentProperties DynatraceEnvironmentProperties { get; set; }
Copy link
Member

@ArcturusZhang ArcturusZhang Nov 14, 2025

Choose a reason for hiding this comment

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

we have quite a few properties here, are all of these properties removed in the new api-version?
Considering this is an update model, it is possible that these properties are never updatable even in the old api-version.
Please give some context about the removal of these properties, so that we could determine a best way of mitigating the breaking change of removing them.

Copy link
Member Author

Choose a reason for hiding this comment

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

If I try to remove this file and execute generateCode command, I get below errors:
image

Comment on lines +18 to +19
/// <summary> user principal id of the user. </summary>
public string UserPrincipal { 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.

I saw in the new generated code, this property shows up on the ctor, which means we should be generating this property which is exactly the same as this.
We could remove the property here and let it generates.

Copy link
Member Author

@arushiarora24 arushiarora24 Nov 14, 2025

Choose a reason for hiding this comment

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

I get below error if I try to remove this file.

image

Comment on lines +13 to +16
/// <summary> Initializes a new instance of <see cref="DynatraceSsoDetailsContent"/>. </summary>
public DynatraceSsoDetailsContent()
{
}
Copy link
Member

Choose a reason for hiding this comment

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

this could be problematic in the future if a collection property is added into this property in the future.
But maybe we could not do anything and it is fine right now, we could leave this here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Noted

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

Labels

Mgmt This issue is related to a management package.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants