Skip to content

telemetry [7]: share background upload transport - #885

Draft
bfjelds (bfjelds) wants to merge 1 commit into
user/bfjelds/mjolnir/telemetry-agent-configfrom
user/bfjelds/mjolnir/telemetry-upload-core
Draft

bfjelds (bfjelds) wants to merge 1 commit into
user/bfjelds/mjolnir/telemetry-agent-configfrom
user/bfjelds/mjolnir/telemetry-upload-core

Conversation

@bfjelds

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

Copy link
Copy Markdown
Member

@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.

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.

🟡 Changes recommended

The changed log exclusion allows uploader shutdown records to produce misleading upload-failure messages after successful shutdown.

1 open issue
What changed in this PR

This PR extracts shared upload infrastructure for log forwarding and the upcoming telemetry uploader, while keeping queue policies separate.

Changes:

  • Centralizes HTTP transport, response validation, redirect protection, and per-origin cooldowns.
  • Shares uploader thread startup and deadline-based shutdown helpers.
  • Updates upload APIs, exports, call sites, and tests.
File Description
crates/​trident/​src/​logging/​upload_core.rs Adds shared transport, cooldown, lifecycle helpers, and tests.
crates/​trident/​src/​logging/​mod.rs Registers the shared upload module.
crates/​trident/​src/​logging/​logstream.rs Passes the new optional content-type argument.
crates/​trident/​src/​logging/​background_uploader.rs Uses shared helpers and adds validation and shutdown APIs.
crates/​trident/​src/​lib.rs Exports BackgroundUploadHandle for reuse.

Review effort: Balanced

Comment thread crates/trident/src/logging/background_uploader.rs Outdated

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 blocking issues remain, and the previously reported log-forwarding feedback issue is resolved with regression coverage.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@bfjelds bfjelds (bfjelds) changed the title telemetry: share background upload transport telemetry [7]: share background upload transport Oct 26, 2026
@bfjelds
bfjelds (bfjelds) force-pushed the user/bfjelds/mjolnir/telemetry-upload-core branch from 67b9659 to c873769 Compare October 26, 2026 01:53
@bfjelds
bfjelds (bfjelds) requested a balanced review from Copilot October 26, 2026 01:56

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

Source review identified no blocking issues, and the prior logging-filter concern is resolved; tests were inspected but not executed.

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

No active review issues block the shared upload-infrastructure extraction.

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 review identifies no active issues with the shared background-upload infrastructure extraction.

No open issues 🎉

Review effort: Balanced

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d0e7cadc-8f44-443f-affe-6ce8e856a218

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

No active review issues remain for the shared background-upload infrastructure extraction.

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