Skip to content

feat(data-service): Improves identification of atlas cluster #2605

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 9 commits into from
Nov 29, 2021
Merged

feat(data-service): Improves identification of atlas cluster #2605

merged 9 commits into from
Nov 29, 2021
+94 −11

Conversation

leorossi
Copy link
Contributor

Description

Checklist

  • New tests and/or benchmarks are included
  • Documentation is changed or added

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)

@leorossi leorossi requested a review from gribnoysup November 22, 2021 09:49
gribnoysup
gribnoysup reviewed Nov 22, 2021
View reviewed changes
mcasimir
mcasimir reviewed Nov 22, 2021
View reviewed changes
mcasimir
mcasimir reviewed Nov 22, 2021
View reviewed changes
@leorossi
Copy link
Contributor Author

Sorry guys I meant to open a draft PR because of many open points 🙏

@gribnoysup
Copy link
Collaborator

Just a reminder not to forget to use this new isAtlas value for the telemetry so we can also address COMPASS-5323 with this PR

@mcasimir
Copy link
Collaborator

Just a reminder not to forget to use this new isAtlas value for the telemetry so we can also address COMPASS-5323 with this PR

We probably want to wait for this to be merged first: #2589

@leorossi leorossi requested a review from gribnoysup November 25, 2021 11:31
gribnoysup
gribnoysup reviewed Nov 25, 2021
View reviewed changes
Copy link
Collaborator

@gribnoysup gribnoysup left a comment

Choose a reason for hiding this comment

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

Looks good to me! Even though old analytics is going away soon, we probably want to also fix it and use the new isAtlas value there too:

compass/packages/compass-metrics/src/modules/rules.js

Line 86 in 200fd6a

is_atlas: !!state.instance._id.match(ATLAS),

addaleax
addaleax approved these changes Nov 25, 2021
View reviewed changes
@leorossi leorossi changed the title Adds isAtlas property to instance detail feat(data-service): Improves identification of atlas cluster Nov 25, 2021
mcasimir
mcasimir reviewed Nov 26, 2021
View reviewed changes
mcasimir
mcasimir approved these changes Nov 26, 2021
View reviewed changes
Copy link
Collaborator

@mcasimir mcasimir left a comment

Choose a reason for hiding this comment

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

nice!

@leorossi leorossi merged commit c118eba into main Nov 29, 2021
@leorossi leorossi deleted the COMPASS-5146 branch November 29, 2021 08:40
@mabaasit mabaasit mentioned this pull request Nov 29, 2021
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gribnoysup gribnoysup gribnoysup left review comments

@mcasimir mcasimir mcasimir approved these changes

@addaleax addaleax addaleax approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@leorossi @gribnoysup @mcasimir @addaleax