feat: improve telemetry performance with fire and forget beacon MONGOSH-3454 - #2802
Closed
nbbeeken wants to merge 19 commits into
Closed
feat: improve telemetry performance with fire and forget beacon MONGOSH-3454#2802nbbeeken wants to merge 19 commits into
nbbeeken wants to merge 19 commits into
Conversation
…H_TELEMETRY_TRANSPORT MONGOSH-3454
…rget transport is selected MONGOSH-3454
…er persistence MONGOSH-3454 TLS 1.3 servers commonly send two NewSessionTickets back to back, and TlsSessionStore.set() replaced this.pendingWrite with a fresh promise chain on every call. That let two unordered writeFile calls race the same path, where the older ticket could win over the newer one. Chain onto the existing pendingWrite instead so writes persist strictly in call order, preserving the silent-failure catch at the end of the chain.
…() ticket grace MONGOSH-3454 ResumingHttpsAgent.awaitingFirstTicket was connectionsCreated > 0 && !ticketCaptured, where connectionsCreated counted createConnection *attempts*. When the telemetry endpoint is down or refuses the connection, no handshake ever completes and no ticket can ever arrive, yet flush() still paid the full 100ms grace period — at exit, in exactly the failure case the circuit breaker exists for. Track handshake completion instead (socket.once('secureConnect', ...)) so the grace is only granted once a handshake has actually completed.
The catch() comment claimed the beacon contract never rejects and that it only guarded gzip/serialization failures, but telemetry-client.spec.ts's own 'stay silent when the beacon rejects despite its contract' test exercises exactly a beacon-contract violation through this same catch. Reword to describe both cases it actually guards.
…e use MONGOSH-3454 this.agent = useOrCreateAgent(this.proxyOptions) in cli-repl.ts is called with no target, and without one useOrCreateAgent always builds an agent — it never returns undefined. That meant setupTelemetryAnalytics always received a defined agent, so FireAndForgetBeacon.agentFor()'s `if (this.options.agent) return this.options.agent;` short-circuited on every send: ResumingHttpsAgent, TlsSessionStore, and the DNS cache were dead code in a real mongosh, and sessionStorePath was silently ignored. Add resolveTelemetryAgent(agent, telemetryEndpoint), which re-resolves the shared agent against the telemetry endpoint via `useOrCreateAgent(agent, telemetryEndpoint, true)`. Verified against node_modules/@mongodb-js/devtools-proxy-support's dist/agent.js: given an existing agent instance, a target, and useTargetRegardlessOfExistingAgent: true, it returns undefined when the agent's own proxyOptions resolve to no proxy for that target, and the agent unchanged otherwise. Use the helper only in the fire-and-forget branch, so the resuming agent (and its TLS session persistence + DNS cache) is only bypassed when a proxy genuinely applies to the telemetry endpoint. Also corrects the now-false doc comment on the `agent` param (it claimed "undefined when no proxy is configured", which no longer — and never really did — hold), and notes that through a proxy agent, dispatched's bytes-reached-kernel guarantee is weaker.
nbbeeken
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.