[AKS] Rename OpenTelemetry ports and add gRPC support for metrics/logs - #10079
[AKS] Rename OpenTelemetry ports and add gRPC support for metrics/logs#10079Christoph Butz (ChFlick) wants to merge 17 commits into
Conversation
❌Azure CLI Extensions Breaking Change Test
|
|
AKS |
12eb037 to
81f1913
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
008c692 to
626a2f2
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
…le flags with deprecated aliases Renames the OTLP CLI parameters on 'az aks create/update' to distinguish the HTTP/protobuf port and to align logs with logs-and-traces, keeping the old names working as deprecated aliases: - --opentelemetry-metrics-port -> --opentelemetry-metrics-port-http - --opentelemetry-logs-port -> --opentelemetry-logs-traces-port-http - --enable-opentelemetry-logs -> --enable-opentelemetry-logs-traces - --disable-opentelemetry-logs -> --disable-opentelemetry-logs-traces Each renamed option is registered as two arguments sharing one destination: the new name carries is_preview=True and the old name is a separate deprecated argument (deprecate_info). This avoids the knack crash from combining is_preview with an option-level c.deprecate (Deprecated has no __len__), and lets the new names show [Preview] while the old names show [Deprecated] and redirect. A validator merges any value supplied via a deprecated alias onto the current destination so downstream logic reads a single dest. Updates help text, validator/decorator error messages, and tests to the new names; migrates the live integration scenarios and fixes stale API field assertions (openTelemetryLogs.port -> openTelemetryLogsAndTraces.httpPort).
Adds --opentelemetry-metrics-port-grpc and --opentelemetry-logs-traces-port-grpc (preview) to 'az aks create/update', wiring the new grpc_port field on the appMonitoring OpenTelemetry profiles (API 2026-04-02-preview). - Validation: all four OTLP ports (metrics/logs x http/grpc) must be in 1..65535 and distinct; grpc ports follow the same enable/disable gating as the http ports. - Disable paths now clear both http_port and grpc_port, including when the parent Azure Monitor metrics/logs are disabled. - Adds unit, validator, and live integration coverage; bumps version to 21.0.0b8 and updates HISTORY.
… and metrics Adds --opentelemetry-*-port-grpc flags and grpcPort assertions to the live OpenTelemetry integration scenarios so gRPC is exercised symmetrically for both the metrics and logs/traces signals (previously only metrics-create asserted grpcPort). The comprehensive monitoring test also now asserts that disabling OpenTelemetry metrics/logs clears both httpPort and grpcPort.
…res header The AzureMonitorAppMonitoringPreview capability is gated by subscription feature registration, not by a per-request AKSHTTPCustomFeatures header. The resource provider now rejects that header with '(BadRequest) Parameter aksHTTPCustomFeatures is not allowed', causing every OpenTelemetry @live_only scenario to fail before reaching its assertions. Removes '--aks-custom-headers AKSHTTPCustomFeatures=Microsoft.ContainerService/AzureMonitorAppMonitoringPreview' from the six OpenTelemetry live tests. Verified live that create/update with the OTLP flags succeeds once the header is dropped (feature registered on the subscription). The recorded app-monitoring tests are left unchanged, as they replay a cassette that still contains the header.
626a2f2 to
5e5c6b1
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
|
Christoph Butz (@ChFlick) are you still working on this PR? |
|
@FumingZhang yes I'm still working on this - I needed to wait for end-to-end testing to be available and I'm waiting for reviews from my team |
The gate guarding ensure_azure_monitor_profile_prerequisites in update_azure_monitor_profile treated every OpenTelemetry parameter as metrics-related. Setting only an OTLP logs/traces port therefore ran the full Prometheus onboarding path, creating an Azure Monitor Workspace, DCE, DCR, DCRA, a Grafana link and recording rules. OpenTelemetry logs and traces ride the Container Insights pipeline and have no dependency on those artifacts, so restrict the gate to Azure Monitor metrics and OpenTelemetry metrics parameters only. Add five regression tests that assert on the prerequisites mock. The existing OpenTelemetry tests patch it purely to suppress network calls and never assert on it, which is why this went unnoticed. Three cover the logs/traces cases and two guard against over-correcting by verifying that metrics-only and mixed-signal commands still onboard exactly once.
Resolves the aks-preview version collision: main released 21.0.0b11
("Vendor new SDK and bump API version to 2026-05-02-preview") while this
branch was also using 21.0.0b11. Moved this branch's changelog entries to a
new 21.0.0b12 section and bumped setup.py accordingly.
Verified the newly vendored SDK still exposes http_port and grpc_port on
ManagedClusterAzureMonitorProfileAppMonitoringOpenTelemetryMetrics and
...OpenTelemetryLogsAndTraces (serialized as httpPort/grpcPort), so the
OpenTelemetry port work on this branch remains compatible.
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
…tlp-grpc # Conflicts: # src/aks-preview/HISTORY.rst
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
The OpenTelemetry live tests enabled Azure Monitor logs and metrics without naming a workspace, so both fell back to the subscription-wide defaults (DefaultWorkspace-<sub>-<region> and DefaultAzureMonitorWorkspace-<region>). Under PARALLELISM=8 concurrent tests race to create/update those shared singletons, producing '(Conflict) Workspace cannot be updated while current provisioning state is not Succeeded' - observed on two tests in ADO build 174587325. Each affected test now provisions its own Log Analytics workspace and Azure Monitor workspace in its own resource group, and passes --workspace-resource-id / --azure-monitor-workspace-resource-id at every enablement site. Adds a _create_azure_monitor_workspace helper mirroring the existing _create_log_analytics_workspace. Workspace assertions now compare the returned logAnalyticsWorkspaceResourceID against the dedicated workspace id (case-insensitive) instead of merely asserting it exists, so a silent fallback to the shared default workspace can no longer pass.
The logs/traces examples passed 4317 to --opentelemetry-logs-traces-port-http, but 4317 is the OTLP gRPC port by convention (4318 is HTTP). Mapping the canonical gRPC port to the HTTP flag works against the goal of making the HTTP-vs-gRPC distinction unambiguous. Uses distinct, convention-neutral ports and shows the paired gRPC flag, matching the style of the metrics examples.
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
Reverts the test-side change in 13c8f97, which dropped --aks-custom-headers AKSHTTPCustomFeatures=Microsoft.ContainerService/AzureMonitorAppMonitoringPreview from the six OpenTelemetry live tests after the RP answered '(BadRequest) Parameter aksHTTPCustomFeatures is not allowed'. That rejection turns out to be scoped to the subscription used for the manual verification, not to the feature or the API. Probing a non-CI subscription reproduces the same BadRequest for an unrelated feature (Microsoft.ContainerService/LocalDNSPreview), i.e. the parameter is refused wholesale there, so the observation says nothing about AzureMonitorAppMonitoringPreview specifically. The live-test subscription behaves differently: ADO build 172295593 (2026-07-15) ran test_aks_automatic_sku_with_hosted_system_enabled, which passes AKSHTTPCustomFeatures for AutomaticSKUPreview and AKS-AutomaticHostedSystemProfilePreview, and reported all tests passed. Removing the header is therefore a plausible cause of the PreviewFeatureNotRegistered failures in build 174587325. Restoring it is cheap to evaluate: the six tests already fail, so a re-run can only add information. If they still report PreviewFeatureNotRegistered, the feature needs registering on the test subscription and this commit can be reverted on its own.
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
The Azure CLI Extensions Breaking Change Test reported 8 x rule 1010 (ParaPropUpdate) failures, one per renamed OpenTelemetry flag on aks create and aks update. azure-cli-diff-tool tracks a parameter by its dest and treats the option list as breaking unless the old options remain a subset of the new ones (meta_change_detect.py: set(base_val).issubset(set(cmp_val))). Registering the previous option names as separate *_deprecated dests meant each original dest lost its original option, so every rename was flagged as a break even though the old flags still worked. Keep a single dest per flag and pass the previous name as a Deprecated entry inside options_list, the idiomatic pattern already used for --agent-pool-name in this file. The exported metadata now lists both option names on the original dest plus options_deprecate_info, which the tool reports as non-breaking. Note: is_preview cannot be combined with a Deprecated entry in options_list, because knack _handle_previews computes sorted(options_list, key=len) and Deprecated has no __len__. The four renamed flags therefore drop is_preview; the two genuinely new gRPC flags keep it. Preview status is not part of the metadata the breaking change tool compares. Because argparse now writes both option names to the same dest, the _merge_opentelemetry_deprecated_aliases validator helper, the eight *_deprecated command parameters and the alias merge tests are all redundant and removed.
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR updates the aks-preview extension’s OpenTelemetry (OTLP) surface area for AKS by renaming the existing OTLP port flags to explicitly indicate HTTP, adding new gRPC port flags for metrics and logs/traces, and ensuring OTLP logs/traces arguments no longer accidentally trigger Prometheus (Azure Monitor Workspace) onboarding.
Changes:
- Introduce
--opentelemetry-*-port-grpcflags and rename existing OTLP port/enable/disable flags with deprecated aliases retained. - Update managed cluster decorator logic to support HTTP+gRPC port payload fields and to gate Prometheus prerequisites only on metrics-related signals.
- Expand unit/live tests and help text to cover the new flags, alias behavior, and the Prometheus onboarding gating.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/aks-preview/setup.py | Bumps extension version to 21.0.0b13. |
| src/aks-preview/linter_exclusions.yml | Adds linter exclusions for newly introduced/renamed OTLP-related flags. |
| src/aks-preview/HISTORY.rst | Documents OTLP flag renames, new gRPC ports, and the Prometheus onboarding gating fix. |
| src/aks-preview/azext_aks_preview/tests/latest/test_validators.py | Extends validator tests for distinct HTTP/gRPC OTLP ports and updated error messages. |
| src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py | Adds decorator tests for gRPC ports and validates the Prometheus prerequisites gating behavior. |
| src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py | Updates live tests to use renamed flags, adds coverage for deprecated aliases, and reduces parallel test races via dedicated workspaces. |
| src/aks-preview/azext_aks_preview/managed_cluster_decorator.py | Implements gRPC port support for metrics/logs+traces and adjusts prerequisites gating to avoid unintended Prometheus onboarding. |
| src/aks-preview/azext_aks_preview/custom.py | Adds new OTLP gRPC port parameters to aks_create / aks_update signatures. |
| src/aks-preview/azext_aks_preview/_validators.py | Validates OTLP HTTP/gRPC ports are in-range and mutually distinct; updates mutual exclusion messages. |
| src/aks-preview/azext_aks_preview/_params.py | Adds renamed options + deprecated aliases and introduces new gRPC port arguments. |
| src/aks-preview/azext_aks_preview/_help.py | Updates parameter help and examples for renamed OTLP flags and gRPC ports. |
Suppressed comments (4)
src/aks-preview/azext_aks_preview/_params.py:1829
- In the
aks updateargument set,enable_opentelemetry_logsis missingis_preview=True, which will drop the [Preview] marker for--enable-opentelemetry-logs-traces. Consider restoringis_preview=Truefor consistency with related OpenTelemetry flags.
c.argument("enable_opentelemetry_logs",
options_list=[
"--enable-opentelemetry-logs-traces",
c.deprecate(
target="--enable-opentelemetry-logs",
redirect="--enable-opentelemetry-logs-traces",
src/aks-preview/azext_aks_preview/_params.py:1806
- In the
aks updateargument set,opentelemetry_metrics_portalso lostis_preview=True, so--opentelemetry-metrics-port-httpwon’t be marked [Preview] while the rest of the OpenTelemetry metrics surface is preview. Addis_preview=Truehere as well.
c.argument("opentelemetry_metrics_port",
options_list=[
"--opentelemetry-metrics-port-http",
c.deprecate(
target="--opentelemetry-metrics-port",
src/aks-preview/azext_aks_preview/_params.py:1839
- In the
aks updateargument set,opentelemetry_logs_portis missingis_preview=True, so--opentelemetry-logs-traces-port-httpwon’t be marked [Preview] while the rest of the OpenTelemetry logs/traces surface is preview. Addis_preview=True.
c.argument("opentelemetry_logs_port",
options_list=[
"--opentelemetry-logs-traces-port-http",
c.deprecate(
target="--opentelemetry-logs-port",
src/aks-preview/azext_aks_preview/_params.py:1857
- In the
aks updateargument set,disable_opentelemetry_logsis missingis_preview=True, which will drop the [Preview] marker for--disable-opentelemetry-logs-traces. Consider restoringis_preview=Truefor consistency with related OpenTelemetry flags.
c.argument("disable_opentelemetry_logs",
options_list=[
"--disable-opentelemetry-logs-traces",
c.deprecate(
target="--disable-opentelemetry-logs",
redirect="--disable-opentelemetry-logs-traces",
| c.argument("enable_opentelemetry_logs", | ||
| options_list=["--enable-opentelemetry-logs"], | ||
| is_preview=True, | ||
| options_list=[ | ||
| "--enable-opentelemetry-logs-traces", | ||
| c.deprecate( | ||
| target="--enable-opentelemetry-logs", |
| c.argument("opentelemetry_logs_port", | ||
| options_list=[ | ||
| "--opentelemetry-logs-traces-port-http", | ||
| c.deprecate( | ||
| target="--opentelemetry-logs-port", | ||
| redirect="--opentelemetry-logs-traces-port-http", | ||
| ), | ||
| ], | ||
| type=int, | ||
| help="HTTP/protobuf port for OpenTelemetry logs and traces collection" | ||
| ) |
| c.argument("disable_opentelemetry_logs", | ||
| is_preview=True, | ||
| options_list=[ | ||
| "--disable-opentelemetry-logs-traces", | ||
| c.deprecate( | ||
| target="--disable-opentelemetry-logs", | ||
| redirect="--disable-opentelemetry-logs-traces", |
| c.argument("opentelemetry_metrics_port", | ||
| options_list=[ | ||
| "--opentelemetry-metrics-port-http", | ||
| c.deprecate( | ||
| target="--opentelemetry-metrics-port", | ||
| redirect="--opentelemetry-metrics-port-http", | ||
| ), | ||
| ], | ||
| type=int, | ||
| help="HTTP/protobuf port for OpenTelemetry metrics collection" | ||
| ) |
There was a problem hiding this comment.
PTAL, please review the other similar comments raised by copilot as well, Christoph Butz (@ChFlick)
There was a problem hiding this comment.
Good catch, the [Preview] tag did get dropped on the four renamed flags.
Looks like its a knack limitation. is_preview=True can't be combined with a deprecated option inside options_list: _handle_previews does sorted(options_list, key=len) (knack/arguments.py:264) and Deprecated has no __len__, so it raises TypeError: object of type 'Deprecated' has no len(). 0.14.0 is the latest knack, so there's no fix to pick up.
I do need the per-option deprecation though. Before that the breaking change test raised errors. The earlier approach (separate *_deprecated params) moved each old option onto a new dest, which the diff tool flags as a breaking change.
So the last two commits add a ~10-line Deprecated subclass that just reports the length of its option name. Everything else still comes from c.deprecate(). Help now shows [Preview] on the new names and [Deprecated] on the old ones, and the breaking change test succeeds.
If you'd rather not subclass a knack internal, the alternative is plain aliases (options_list=["--new", "--old"] + is_preview=True) with a note in the help text. That keeps the preview tags but loses the [Deprecated] tag, the runtime warning and the options_deprecate_info metadata, which I think is less optimal..
| c.argument("opentelemetry_metrics_port", | ||
| options_list=[ | ||
| "--opentelemetry-metrics-port-http", | ||
| c.deprecate( | ||
| target="--opentelemetry-metrics-port", | ||
| redirect="--opentelemetry-metrics-port-http", | ||
| ), | ||
| ], | ||
| type=int, | ||
| help="HTTP/protobuf port for OpenTelemetry metrics collection" | ||
| ) |
There was a problem hiding this comment.
PTAL, please review the other similar comments raised by copilot as well, Christoph Butz (@ChFlick)
| "--location {workspace_location} --query id -o tsv" | ||
| ).output.strip() | ||
|
|
||
| def _create_azure_monitor_workspace(self, resource_group_location): |
There was a problem hiding this comment.
Queued live test to validate the change.
- test_aks_create_with_azuremonitorlogs_and_opentelemetry
- test_aks_update_with_azuremonitorlogs_and_opentelemetry
- test_aks_create_with_azuremonitormetrics_and_opentelemetry
- test_aks_update_with_azuremonitormetrics_and_opentelemetry
- test_aks_create_with_opentelemetry_deprecated_flag_aliases
- test_aks_comprehensive_monitoring_integration
Queue live test for these standard Azure Monitor scenarios to verify there are no regressions.
- test_aks_create_with_azuremonitorlogs
- test_aks_update_with_azuremonitorlogs
- test_aks_create_with_azuremonitormetrics_v2
- test_aks_update_with_azuremonitormetrics_v2
There was a problem hiding this comment.
One of the live tests failed due to a missing waiting condition, should be fixed now
There was a problem hiding this comment.
Re-queued live test for test_aks_update_with_azuremonitorlogs_and_opentelemetry
…teps test_aks_update_with_azuremonitorlogs_and_opentelemetry failed in live run 175416696 with: (CreateOrUpdateExtensionFailed) Create or update of core cluster extension aks-managed-azure-monitor-logs of type microsoft.azuremonitor.containers failed. Update failed for this resource, as there is a conflicting operation in progress. The monitoring cluster extension keeps provisioning after 'az aks update' returns, so the following '--disable-opentelemetry-logs-traces' update collided with the still-running enable operation. Adds 'az aks wait --updated' after the enable update and after the disable update, which is the pattern test_aks_comprehensive_monitoring_integration already uses. That test runs the same disable command and passed in the same run.
The 'aks create' examples used '--enable-azuremonitormetrics', which is deprecated with hide=True and redirects to '--enable-azure-monitor-metrics'. Examples must show the current name. Addresses the review comment on the OpenTelemetry metrics examples. The parameter documentation entries for the deprecated names are unchanged, because those still document the deprecated flags themselves.
The renamed flags lost their [Preview] tag when they moved to per-option deprecation. knack computes the preview target with sorted(options_list, key=len), and a plain Deprecated has no __len__, so is_preview=True raised TypeError when combined with a deprecated option name. Adds a small Deprecated subclass that reports the length of its option name. isinstance checks in _handle_deprecations still match, and sorted() now picks the longest option, which is the current name. Each renamed flag shows [Preview] on the current name and [Deprecated] on the old name. Verified in az aks update --help. The exported metadata is unchanged, so the breaking change test still reports 0 breaking findings.
The helper constructed the Deprecated object directly, which duplicated the message text and skipped the applicability and stale-context checks that c.deprecate() performs. It also ignored the expiration field when building the message. Now the helper calls c.deprecate() and only replaces the class, so the message, the object type and both checks stay identical to every other deprecated option in this file.
…tlp-grpc # Conflicts: # src/aks-preview/HISTORY.rst # src/aks-preview/setup.py
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
🤖 PR Validation — ❌ Action needed
❌Azure CLI Extensions Breaking Change Test
Testing notes:
azdev extension add aks-previewand confirmed it loads & verified new flags appear inaz aks update --helpwith correct [Preview] (new port/enable flags) and [Deprecated] (renamed old flags) decorationaz aks updateagainst a live cluster with--enable-opentelemetry-metrics --opentelemetry-metrics-port-http/-grpcand used--debugto confirm the payload includes the new grpcPort / httpPort fields underazureMonitorProfile.appMonitoring.openTelemetryMetrics--enable-opentelemetry-*flag isn't set / OTLP isn't already enabledThis checklist is used to make sure that common guidelines for a pull request are followed.
Related command
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (azdevrequired; see.azure-pipelines/templates/azdev_setup.ymlfor the install command untilazdev==0.2.11b1is on PyPI)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.