Integrate Application Insights and OpenTelemetry for CLI observability#1838
Conversation
|
Since the CLI is a public package available to anyone, any tracking mechanism must be opt-in and accompanied by a privacy policy. The same applies to pipelines. Alternatively, we must restrict tracking to internal usage only |
3164682 to
bffee13
Compare
restricted to pagopa users only, via github |
bffee13 to
85a4b82
Compare
ecff58e to
0d4b05f
Compare
0d4b05f to
fa45050
Compare
03201bb to
80b449d
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end observability for the dx CLI by integrating Azure Monitor (Application Insights) and OpenTelemetry, including a single root span for each CLI invocation and log export via Logtape → OTel. It also tightens CLI preconditions (GitHub auth) and updates workspace dependency catalogs/lockfiles accordingly.
Changes:
- Add Azure Monitor/OpenTelemetry preload + runtime enablement, root span tracing, and OTel log sink wiring for the CLI.
- Enforce GitHub authentication for all CLI commands except
--help/--version, with telemetry enabled only forpagopaorg members. - Update dependency catalogs/lockfile and documentation to reflect new auth + telemetry behavior.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Adds/updates OpenTelemetry-related catalog entries and normalizes quoting/format. |
| pnpm-lock.yaml | Updates lockfile to include new OTel/Logtape/Azure Monitor dependencies and related transitive changes. |
| package.json | Updates packageManager and adds pnpm overrides at root. |
| apps/website/docs/dx-cli/requirements.md | Documents mandatory GitHub auth and telemetry enablement rules. |
| apps/cli/src/index.ts | Adds early auth/telemetry gating, OTel log sink, root span lifecycle, and tool-version enrichment. |
| apps/cli/src/adapters/plop/actions/setup-pnpm.ts | Strips debugger-related env vars from spawned processes. |
| apps/cli/src/adapters/octokit/index.ts | Adds isPagopaOrgMember helper to gate telemetry. |
| apps/cli/src/adapters/octokit/tests/index.test.ts | Adds unit tests for isPagopaOrgMember. |
| apps/cli/src/adapters/commander/commands/doctor.ts | Avoids process.exit() to allow telemetry flush in finally. |
| apps/cli/src/adapters/commander/commands/add.ts | Extends action result shape to include the environment payload. |
| apps/cli/src/adapters/azure-monitor/telemetry.ts | Adds flushTelemetry() helper to shutdown/flush Azure Monitor SDK safely. |
| apps/cli/src/adapters/azure-monitor/instrumentation.ts | Adds preload module hook registration + Azure Monitor enablement and resource attribute setup. |
| apps/cli/README.md | Documents GitHub auth requirement and telemetry scope. |
| apps/cli/package.json | Adds Azure Monitor + OTel deps and Logtape OTel sink deps to the CLI app. |
| apps/cli/bin/index.js | Ensures instrumentation preload is imported before the CLI entrypoint via sequential dynamic imports. |
| .nx/version-plans/version-plan-1781270390103.md | Version plan for docs change (GitHub login requirement). |
| .nx/version-plans/version-plan-1781270291055.md | Version plan for CLI telemetry behavior change. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
| process.env["OTEL_SERVICE_NAME"] = "dx-cli"; | ||
| process.env["ATTR_SERVICE_NAMESPACE"] = "dx"; | ||
|
|
| opentelemetry: | ||
| '@azure/monitor-opentelemetry': 1.18.1 | ||
| '@azure/monitor-opentelemetry-exporter': ^1.0.0-beta.42 | ||
| '@opentelemetry/api': ^1.9.1 | ||
| '@opentelemetry/api-logs': ^0.218.0 | ||
| '@opentelemetry/instrumentation': ^0.218.0 | ||
| '@opentelemetry/instrumentation-undici': ^0.28.0 | ||
| '@opentelemetry/resources': ^2.7.1 | ||
| '@opentelemetry/sdk-metrics': ^2.7.1 | ||
| '@opentelemetry/sdk-trace-base': ^2.7.1 | ||
| "@azure/monitor-opentelemetry": 1.18.1 | ||
| "@azure/monitor-opentelemetry-exporter": ^1.0.0-beta.42 | ||
| "@logtape/otel": 1.3.8 | ||
| "@opentelemetry/api": ^1.9.1 | ||
| "@opentelemetry/api-logs": ^0.218.0 | ||
| "@opentelemetry/instrumentation": ^0.218.0 | ||
| "@opentelemetry/instrumentation-undici": ^0.28.0 | ||
| "@opentelemetry/resources": ^2.7.1 | ||
| "@opentelemetry/sdk-metrics": ^2.7.1 | ||
| "@opentelemetry/sdk-trace-base": ^2.7.1 | ||
| "@opentelemetry/sdk-logs": ^0.218.0 |
| const otelSink = getOpenTelemetrySink({ | ||
| loggerProvider: logs.getLoggerProvider() as unknown as LoggerProvider, | ||
| }); |
94ef95f to
6f50e56
Compare
6f50e56 to
1ab8c3b
Compare
1ab8c3b to
16ac30e
Compare
16ac30e to
b1cfcc6
Compare
| azureMonitorExporterOptions: { | ||
| connectionString: | ||
| process.env.APPLICATIONINSIGHTS_CONNECTION_STRING || | ||
| "InstrumentationKey=e0ff8094-78fa-45e5-a21d-e62b453dc5d1;IngestionEndpoint=https://italynorth-0.in.applicationinsights.azure.com/;LiveEndpoint=https://italynorth.livediagnostics.monitor.azure.com/;ApplicationId=ce469d55-2ff7-4dfd-a249-cc787291e672", |
There was a problem hiding this comment.
The Application Insights connection string is currently hardcoded into the public npm package. Although the CLI only utilizes it for PagoPA members, anyone could extract it and send spoofed telemetry data directly to that endpoint. Is this an accepted risk, or would we prefer to inject it at build or runtime?
There was a problem hiding this comment.
Accepted risk, we can rotate or block connections at any time. Moreover we have a daily cap
There was a problem hiding this comment.
note: we could use managed identity. I did some experiments with that a few weeks ago, and the steps to use Managed Identity should be:
- force our Application Insights to use Managed Identity by adding
local_authentication_disabled = true - add
Monitoring Metrics Publisherrole to the instances that need to push telemetry data - use the
@pagopa/azure-tracingto initialize Azure Monitor using the Managed Identity (the package already supports Managed Identity authentication)
Please note that with this current approach, the Connection String will still be publicly available, but it won't be enough to send data.
There was a problem hiding this comment.
if it is not possible to use the @pagopa/azure-tracing package, then we should add here the logic to use the Managed Identity instead (through the @azure/identity package)
There was a problem hiding this comment.
The issue in using IAM is that dx cli is a client app, so it doesn't have any role but the user identity. This requires giving access to all the users.
An alternative is using service principals, but it requires saving a secret locally
b1cfcc6 to
c2de757
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-mud-0405c6703-1838.westeurope.1.azurestaticapps.net |



Implement OpenTelemetry logging for improved observability, including root span tracing for CLI commands. Enhance telemetry capabilities by concurrently fetching tool versions and adding Azure Monitor service name.