Skip to content

telemetry [18]: configure telemetry ingestion destinations - #886

Draft
bfjelds (bfjelds) wants to merge 1 commit into
user/bfjelds/mjolnir/appinsights-telemetryfrom
user/bfjelds/mjolnir/telemetry-connection-strings
Draft

bfjelds (bfjelds) wants to merge 1 commit into
user/bfjelds/mjolnir/appinsights-telemetryfrom
user/bfjelds/mjolnir/telemetry-connection-strings

Conversation

@bfjelds

@bfjelds bfjelds (bfjelds) commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Pass the CI connection string through Docker build arguments and RPM macros.
  • Distro RPM builds use the public-usage string; repository RPM builds use the supplied override or an empty string.

Validation

At 8be95efefec3a13e8dcca8ca736d145355265a00 on 2026-10-30:

  • cargo fmt -- --check: passed.
  • All-target compilation and strict Clippy (-D warnings): passed with grpc-preview,dangerous-options,trident_api/documentation,functional-test,pytest-generator.
  • Focused host-safe unit tests: passed without enabling functional-test.
  • Targeted daemon and gRPC-client unit tests: passed.
  • Binary pre-reboot flush regression: passed.
  • VM functional tests were not run as part of this local validation. No CI result is asserted here.

Stack

Native stack 913, position 18/20. Positions 1-14 provide sink-neutral telemetry; positions 15-20 add telemetry preferences, shared transport, AppInsights and build/image/package integration.

Full-stack pipeline branch: user/bfjelds/telemetry-default-optin (#910).

Related PRs in stack:

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@bfjelds
bfjelds (bfjelds) removed this pull request from stack #887 October 26, 2026 00:55
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 26, 2026 01:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

RPM handling of Docker’s empty default connection string remains unvalidated and needs confirmation before approval.

No open issues 🎉

What changed in this PR

This PR wires build-time telemetry destinations into Trident’s existing telemetry support, separating CI-built artifacts from Azure Linux distro packages.

Changes:

  • Pass the CI telemetry connection string through Docker to RPM builds.
  • Configure a separate destination for distro builds.
  • Preserve an empty connection string when the optional repo-build RPM macro is absent.
File Description
packaging/​rpm/​trident.spec Selects and exports the build-time telemetry connection string.
packaging/​docker/​Dockerfile.full Forwards the connection string as an RPM macro.
.pipelines/​templates/​stages/​trident_rpms/​release.yml Supplies the CI telemetry destination to Docker builds.

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-connection-strings branch from ff6638c to 1092d40 Compare October 26, 2026 01:33
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 26, 2026 01:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

RPM handling of Docker’s empty default connection-string argument remains unverified, leaving local no-telemetry builds uncertain.

No open issues 🎉

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The changes separating CI and distro telemetry destinations have no active review issues.

No open issues 🎉

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-connection-strings branch from 423f75d to 260261c Compare October 30, 2026 18:52
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 30, 2026 18:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are identified for the build-time telemetry configuration changes.

No open issues 🎉

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-connection-strings branch from 260261c to 78dd3f6 Compare October 30, 2026 19:25
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 30, 2026 19:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain for the build-time telemetry configuration changes.

No open issues 🎉

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-connection-strings branch from 78dd3f6 to 15b3f7c Compare October 30, 2026 19:51
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 30, 2026 19:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are reported for the build-time telemetry configuration changes.

No open issues 🎉

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-connection-strings branch from 15b3f7c to 3e43a9a Compare October 30, 2026 20:14
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 30, 2026 20:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are identified for the build-time telemetry configuration changes.

No open issues 🎉

Review effort: Balanced

@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-connection-strings branch 2 times, most recently from 8f78bad to e617c47 Compare October 30, 2026 22:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are reported for the build-time telemetry configuration changes.

No open issues 🎉

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

There are no active review issues for the build-time telemetry configuration changes.

No open issues 🎉

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

There are no active review issues for the build-time telemetry configuration changes.

No open issues 🎉

Review effort: Balanced

Rebase onto agent configuration read-error handling.

Original layer head: 8be95ef

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 43cb811b-f191-4d90-852f-e3373041ca11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The build-time telemetry destination changes have no active review issues.

No open issues 🎉

Review effort: Balanced

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants