Skip to content

Conversation

syn-zhu
Copy link
Collaborator

@syn-zhu syn-zhu commented Aug 26, 2025

Description

Moving the log line before the segment event tracking call

Checklist

  • New tests and/or benchmarks are included
  • Documentation is changed or added
  • If this change updates the UI, screenshots/videos are added and a design review is requested
  • I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@Copilot Copilot AI review requested due to automatic review settings August 26, 2025 19:43
@syn-zhu syn-zhu requested a review from a team as a code owner August 26, 2025 19:43
@syn-zhu syn-zhu requested a review from nbbeeken August 26, 2025 19:43
@github-actions github-actions bot added the fix label Aug 26, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves a log statement for connection success earlier in the connection flow to improve connection latency tracing for Data Explorer. The change relocates the log from after tracking and other post-connection operations to immediately after connection validation.

  • Relocates the "Compass Connection Attempt Succeeded" log statement earlier in the connection flow
  • Improves timing accuracy for Data Explorer connection latency tracing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@syn-zhu syn-zhu added the no release notes Fix or feature not for release notes label Aug 26, 2025
@syn-zhu syn-zhu assigned nbbeeken and unassigned nbbeeken Aug 26, 2025
@@ -1792,6 +1792,17 @@ const connectWithOptions = (
);
}

// This is used for Data Explorer connection latency tracing
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you clarify and document why it's a fix and why it's important that this line is exactly here? Without this chances are high somebody will just move it again by accident

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, ideally I'm trynna see if I could just cover this with a test haha

@syn-zhu syn-zhu marked this pull request as draft August 27, 2025 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix no release notes Fix or feature not for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants