Add automation to report on non-released SDKs #13333
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces automation to report on non-released Azure SDKs by sending email notifications to release plan owners when Tier 1 SDK coverage is missing. The solution queries overdue release plans via the Azure SDK CLI, identifies missing Tier 1 language SDKs, and automatically sends email notifications with action items.
Key Changes:
- New PowerShell script to send email notifications via Azure SDK email service API
- New automation script to check release plans for missing Tier 1 SDKs and notify owners
- New Azure DevOps pipeline to execute the reporting automation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 17 comments.
| File | Description |
|---|---|
| eng/scripts/Send-Email-Notification.ps1 | Utility script that sends email notifications by posting to Azure SDK email service REST API |
| eng/scripts/Report-Unreleased-Sdks.ps1 | Main automation script that identifies missing Tier 1 SDKs in overdue release plans and sends notifications to owners |
| eng/pipelines/report-unreleased-sdks.yml | Azure DevOps pipeline configuration to run the unreleased SDK reporting automation |
… report-unreleased-sdks
tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/ReleasePlan/ReleasePlanTool.cs
Outdated
Show resolved
Hide resolved
| if (notifyOwners && string.IsNullOrWhiteSpace(emailerUri)) | ||
| { | ||
| return new ReleasePlanListResponse { ResponseError = "Emailer URI is required when notify owners is enabled." }; | ||
| } |
There was a problem hiding this comment.
Missing input validation for the emailerUri parameter. When notifyOwners is true, the method only checks if emailerUri is null or whitespace, but doesn't validate if it's a valid URI format. Consider adding URI validation using Uri.TryCreate to ensure the emailerUri is well-formed before attempting to use it in the HTTP POST request.
| } | |
| } | |
| if (notifyOwners) | |
| { | |
| if (!Uri.TryCreate(emailerUri, UriKind.Absolute, out var validatedUri)) | |
| { | |
| return new ReleasePlanListResponse { ResponseError = "Emailer URI is not a valid absolute URI." }; | |
| } | |
| } |
| // Identify missing Tier 1 languages | ||
| var missingSDKs = new List<string> { ".NET", "JavaScript", "Python", "Java", "Go" }; | ||
| // Skip Go for Data Plane release plans | ||
| if (releasePlan.IsDataPlane) | ||
| { | ||
| missingSDKs.Remove("Go"); | ||
| } |
There was a problem hiding this comment.
The hardcoded Tier 1 languages list should be documented or extracted to a constant. The list includes ".NET", "JavaScript", "Python", "Java", "Go" and has special logic for Go being excluded from Data Plane. This business logic would be more maintainable and testable if extracted to a constant or configuration, with inline documentation explaining the Data Plane exclusion rule for Go.
tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/ReleasePlan/ReleasePlanTool.cs
Show resolved
Hide resolved
| var body = $""" | ||
| <html> | ||
| <body> | ||
| <p>Hello {releaseOwnerName},</p> | ||
| <p>Our automation has flagged your Azure SDK release plan as missing one or more Tier 1 SDKs for the following plane:</p> | ||
| <ul> | ||
| <li><strong>Plane:</strong> {plane}</li> | ||
| <li><strong>Missing SDKs:</strong> {string.Join(", ", missingSDKs)}</li> | ||
| <li><strong>Release Plan Link:</strong> <a href={releasePlanLink}>{releasePlanLink}</a></li> | ||
| </ul> | ||
| <p>Per Azure SDK release requirements, all Tier 1 languages must be supported unless an approved exclusion is filed. Please take one of the following actions:</p> | ||
| <ol> | ||
| <li>Generate and release the missing SDKs using <a href='https://aka.ms/azsdk/dpcodegen'>https://aka.ms/azsdk/dpcodegen</a></li> | ||
| <li>File for an exclusion: <a href='https://eng.ms/docs/products/azure-developer-experience/onboard/request-exception'>https://eng.ms/docs/products/azure-developer-experience/onboard/request-exception</a></li> | ||
| </ol> | ||
| <p>Thank you for helping maintain language parity across Azure SDKs.</p> | ||
| <p>Best regards,<br/>Azure SDK PM Team</p> | ||
| </body> | ||
| </html> | ||
| """; |
There was a problem hiding this comment.
The HTML email body constructs dynamic content without proper HTML escaping, which could lead to malformed HTML or potential XSS vulnerabilities if user-controlled data contains HTML special characters. Variables like releaseOwnerName, plane, releasePlanLink, and the missingSDKs list should be HTML-encoded before being inserted into the HTML template. Consider using a library like System.Net.WebUtility.HtmlEncode for proper escaping.
| foreach (var releasePlan in releasePlans) | ||
| { | ||
| var releaseOwnerEmail = releasePlan.ReleasePlanSubmittedByEmail; | ||
|
|
||
| // Validate email address | ||
| if (string.IsNullOrWhiteSpace(releaseOwnerEmail) || !Regex.IsMatch(releaseOwnerEmail, @"^[^@\s]+@[^@\s]+\.[^@\s]+$", RegexOptions.IgnoreCase)) | ||
| { | ||
| logger.LogWarning("Skipped notification for Release Plan ID {WorkItemId}: invalid email '{Email}'", | ||
| releasePlan.WorkItemId, releaseOwnerEmail); | ||
| continue; | ||
| } | ||
|
|
||
| var releaseOwnerName = releasePlan.Owner; | ||
| var plane = releasePlan.IsManagementPlane ? "Management Plane" : "Data Plane"; | ||
| var releasePlanLink = releasePlan.ReleasePlanLink; | ||
|
|
||
| // Identify missing Tier 1 languages | ||
| var missingSDKs = new List<string> { ".NET", "JavaScript", "Python", "Java", "Go" }; | ||
| // Skip Go for Data Plane release plans | ||
| if (releasePlan.IsDataPlane) | ||
| { | ||
| missingSDKs.Remove("Go"); | ||
| } | ||
| // Remove languages that are already released | ||
| foreach (var info in releasePlan.SDKInfo) | ||
| { | ||
| if (string.Equals(info.ReleaseStatus, "released", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| missingSDKs.RemoveAll(lang => string.Equals(lang, info.Language, StringComparison.OrdinalIgnoreCase)); | ||
| } | ||
| } | ||
|
|
||
| var body = $""" | ||
| <html> | ||
| <body> | ||
| <p>Hello {releaseOwnerName},</p> | ||
| <p>Our automation has flagged your Azure SDK release plan as missing one or more Tier 1 SDKs for the following plane:</p> | ||
| <ul> | ||
| <li><strong>Plane:</strong> {plane}</li> | ||
| <li><strong>Missing SDKs:</strong> {string.Join(", ", missingSDKs)}</li> | ||
| <li><strong>Release Plan Link:</strong> <a href={releasePlanLink}>{releasePlanLink}</a></li> | ||
| </ul> | ||
| <p>Per Azure SDK release requirements, all Tier 1 languages must be supported unless an approved exclusion is filed. Please take one of the following actions:</p> | ||
| <ol> | ||
| <li>Generate and release the missing SDKs using <a href='https://aka.ms/azsdk/dpcodegen'>https://aka.ms/azsdk/dpcodegen</a></li> | ||
| <li>File for an exclusion: <a href='https://eng.ms/docs/products/azure-developer-experience/onboard/request-exception'>https://eng.ms/docs/products/azure-developer-experience/onboard/request-exception</a></li> | ||
| </ol> | ||
| <p>Thank you for helping maintain language parity across Azure SDKs.</p> | ||
| <p>Best regards,<br/>Azure SDK PM Team</p> | ||
| </body> | ||
| </html> | ||
| """; | ||
|
|
||
| await SendEmailNotification(emailerUri, releaseOwnerEmail, sdkApexEmail, subject, body); | ||
| } |
There was a problem hiding this comment.
The NotifyOwnersOfOverdueReleasePlans method sends emails sequentially in a foreach loop, which could be slow if there are many overdue release plans. Consider implementing parallel email sending with appropriate rate limiting, or at minimum batching the notifications to improve performance while avoiding overwhelming the email service.
| var releaseOwnerEmail = releasePlan.ReleasePlanSubmittedByEmail; | ||
|
|
||
| // Validate email address | ||
| if (string.IsNullOrWhiteSpace(releaseOwnerEmail) || !Regex.IsMatch(releaseOwnerEmail, @"^[^@\s]+@[^@\s]+\.[^@\s]+$", RegexOptions.IgnoreCase)) |
There was a problem hiding this comment.
The email validation regex is overly simplistic and may allow invalid email addresses. The current pattern ^[^@\s]+@[^@\s]+\.[^@\s]+$ doesn't properly validate email addresses according to RFC standards. Consider using a more robust email validation approach, such as MailAddress parsing with try-catch, or a more comprehensive regex pattern that handles edge cases like multiple @ symbols, consecutive dots, or invalid domain names.
| catch (Exception ex) | ||
| { | ||
| logger.LogError(ex, "Failed to send email. To: {To}, CC: {CC}, Subject: {Subject}", to, cc, subject); | ||
| } | ||
| } |
There was a problem hiding this comment.
The SendEmailNotification method silently catches and logs all exceptions without propagating them or affecting the overall command response. This means if email sending fails for all release plans, the ListOverdueReleasePlans method will still return a success response with no indication that notifications failed. Consider collecting email failures and including them in the response, or at minimum, logging a summary of how many emails succeeded vs. failed.
| foreach (var info in releasePlan.SDKInfo) | ||
| { | ||
| if (string.Equals(info.ReleaseStatus, "released", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| missingSDKs.RemoveAll(lang => string.Equals(lang, info.Language, StringComparison.OrdinalIgnoreCase)); | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| var httpContent = new StringContent(jsonContent, Encoding.UTF8, "application/json"); | ||
|
|
||
| logger.LogInformation("Sending Email - To: {To}, CC: {CC}, Subject: {Subject}", to, cc, subject); | ||
|
|
||
| var response = await httpClient.PostAsync(emailerUri, httpContent); | ||
| response.EnsureSuccessStatusCode(); | ||
|
|
||
| logger.LogInformation("Successfully sent email - To: {To}, CC: {CC}, Subject: {Subject}", to, cc, subject); |
There was a problem hiding this comment.
Disposable 'StringContent' is created but not disposed.
| var httpContent = new StringContent(jsonContent, Encoding.UTF8, "application/json"); | |
| logger.LogInformation("Sending Email - To: {To}, CC: {CC}, Subject: {Subject}", to, cc, subject); | |
| var response = await httpClient.PostAsync(emailerUri, httpContent); | |
| response.EnsureSuccessStatusCode(); | |
| logger.LogInformation("Successfully sent email - To: {To}, CC: {CC}, Subject: {Subject}", to, cc, subject); | |
| using (var httpContent = new StringContent(jsonContent, Encoding.UTF8, "application/json")) | |
| { | |
| logger.LogInformation("Sending Email - To: {To}, CC: {CC}, Subject: {Subject}", to, cc, subject); | |
| var response = await httpClient.PostAsync(emailerUri, httpContent); | |
| response.EnsureSuccessStatusCode(); | |
| logger.LogInformation("Successfully sent email - To: {To}, CC: {CC}, Subject: {Subject}", to, cc, subject); | |
| } |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
|
||
| private async Task NotifyOwnersOfOverdueReleasePlans(List<ReleasePlanDetails> releasePlans, string emailerUri) | ||
| { | ||
| const string subject = "Action Required: Missing Tier 1 language in your Azure SDK Release Plan"; |
There was a problem hiding this comment.
We are notifying users about their pending release(past due release) and not about missing SDK in release plan. right?
There was a problem hiding this comment.
@MrJustinB created an email template he wanted to use in the issue: #12059
There was a problem hiding this comment.
@MrJustinB This change detects the overdue release plans based on the release month selected in the release plan. So it is not only about the languages opted out in the release plans. It will detect the release plans that were supposed to be completed but not released for all required languages yet. I think we should update the email template to clarify the message.
| // Remove languages that are already released | ||
| foreach (var info in releasePlan.SDKInfo) | ||
| { | ||
| if (string.Equals(info.ReleaseStatus, "released", StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
We should also skip the languages which are marked as excluded. This information is available in language specific fields in the work item. We should ignore all languages with exclusion status requested/approved.
ReleaseExclusionStatusForPython
ReleaseExclusionStatusForDotnet
ReleaseExclusionStatusForJava
ReleaseExclusionStatusForJavaScript
ReleaseExclusionStatusForGo
| var missingSDKs = new List<string> { ".NET", "JavaScript", "Python", "Java", "Go" }; | ||
| // Skip Go for Data Plane release plans | ||
| if (releasePlan.IsDataPlane) | ||
| { | ||
| missingSDKs.Remove("Go"); | ||
| } |
There was a problem hiding this comment.
I think it's better not to handle the list here. It will easily be forgotten that we have a list maintained here too. Instead, we can just rely on the SDKInfo list in the release plan.
releasePlan.SDKInfo.Where(s => !string.Equals(s.ReleaseStatus, "released", StringComparison.OrdinalIgnoreCase))
|
@maririos @MrJustinB Current approach identifies the list of past due release plans and will send an email notification to release plan owner. It will send an email notification to users until they get the SDK as released or excluded. We can schedule this pipeline to run weekly so it's less aggressive but act as a reminder email weekly to users with past due release plans. Alternate solution is to add a field in work item to check if the notification was already sent or not, so we send the email only once per release plan. Please share your thoughts on this solution. I am in favor of repeated weekly notification which act as a reminder to complete the past due release plans. |
|
@praveenkuttappan @maririos I am ok with weekly, but I am wondering if I misunderstood the feature when I drafted the email template. This PR is to notify teams that have decided to have SDKs for all tier 1 languages BUT they have not released them yet? If so, I should update the template to match that scenario. However, if we're adding self-serve to release planner for data plane, aren't the SDKs released automatically on behalf of the service teams? If so, does that make this PR obsolete? If not, can you please help me understand the specific scenario with self-serve enabled? |
5 participants
Resolves Issue: #12059