Skip to content

fix: track usage of uuid subtype 3 vs 4 COMPASS-9359 #6943

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

Merged
merged 5 commits into from
May 28, 2025
Merged

fix: track usage of uuid subtype 3 vs 4 COMPASS-9359 #6943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
53e3259
fix: track usage of uuid subtype 3 vs 4 COMPASS-9359
paula-stacho May 21, 2025
7a31f94
cleanup
paula-stacho May 21, 2025
dbf57c1
use callback
paula-stacho May 22, 2025
1cf3098
traverse docs
paula-stacho May 22, 2025
bcf88a6
update test
paula-stacho May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/compass-components/src/components/bson-value.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ const BinaryValue: React.FunctionComponent<PropsByValueType<'Binary'>> = ({
}
if (value.sub_type === Binary.SUBTYPE_UUID) {
let uuid: string;

try {
// Try to get the pretty hex version of the UUID
uuid = value.toUUID().toString();
Expand Down
12 changes: 7 additions & 5 deletions packages/compass-crud/src/components/readonly-document.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,13 @@ class ReadonlyDocument extends React.Component<
*/
renderElements() {
return (
<DocumentList.Document
value={this.props.doc}
// Provide extra whitespace for the expand button
extraGutterWidth={spacing[900]}
/>
<>
<DocumentList.Document
value={this.props.doc}
// Provide extra whitespace for the expand button
extraGutterWidth={spacing[900]}
/>
</>
);
}

Expand Down
30 changes: 24 additions & 6 deletions packages/compass-crud/src/stores/crud-store.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2296,6 +2296,7 @@ describe('store', function () {
});

describe('fetchDocuments', function () {
const track = createNoopTrack();
let findResult: unknown[] = [];
let csfleMode = 'disabled';
let find = sinon.stub().callsFake(() => {
Expand Down Expand Up @@ -2323,7 +2324,7 @@ describe('store', function () {
});

it('should call find with $bsonSize projection when mongodb version is >= 4.4, not connected to ADF and csfle is disabled', async function () {
await fetchDocuments(dataService, '5.0.0', false, 'test.test', {});
await fetchDocuments(dataService, track, '5.0.0', false, 'test.test', {});
expect(find).to.have.been.calledOnce;
expect(find.getCall(0))
.to.have.nested.property('args.2.projection')
Expand All @@ -2334,6 +2335,7 @@ describe('store', function () {
findResult = [{ __size: new Int32(42), __doc: { _id: 1 } }];
const docs = await fetchDocuments(
dataService,
track,
'4.0.0',
false,
'test.test',
Expand All @@ -2345,7 +2347,7 @@ describe('store', function () {
});

it('should NOT call find with $bsonSize projection when mongodb version is < 4.4', async function () {
await fetchDocuments(dataService, '4.0.0', false, 'test.test', {});
await fetchDocuments(dataService, track, '4.0.0', false, 'test.test', {});
expect(find).to.have.been.calledOnce;
expect(find.getCall(0)).to.have.nested.property(
'args.2.projection',
Expand All @@ -2354,7 +2356,7 @@ describe('store', function () {
});

it('should NOT call find with $bsonSize projection when connected to ADF', async function () {
await fetchDocuments(dataService, '5.0.0', true, 'test.test', {});
await fetchDocuments(dataService, track, '5.0.0', true, 'test.test', {});
expect(find).to.have.been.calledOnce;
expect(find.getCall(0)).to.have.nested.property(
'args.2.projection',
Expand All @@ -2364,7 +2366,7 @@ describe('store', function () {

it('should NOT call find with $bsonSize projection when csfle is enabled', async function () {
csfleMode = 'enabled';
await fetchDocuments(dataService, '5.0.0', false, 'test.test', {});
await fetchDocuments(dataService, track, '5.0.0', false, 'test.test', {});
expect(find).to.have.been.calledOnce;
expect(find.getCall(0)).to.have.nested.property(
'args.2.projection',
Expand All @@ -2375,6 +2377,7 @@ describe('store', function () {
it('should keep user projection when provided', async function () {
await fetchDocuments(
dataService,
track,
'5.0.0',
false,
'test.test',
Expand All @@ -2399,6 +2402,7 @@ describe('store', function () {

const docs = await fetchDocuments(
dataService,
track,
'5.0.0',
false,
'test.test',
Expand All @@ -2419,7 +2423,14 @@ describe('store', function () {
find = sinon.stub().rejects(new TypeError('🤷‍♂️'));

try {
await fetchDocuments(dataService, '5.0.0', false, 'test.test', {});
await fetchDocuments(
dataService,
track,
'5.0.0',
false,
'test.test',
{}
);
expect.fail('Expected fetchDocuments to fail with error');
} catch (err) {
expect(find).to.have.been.calledOnce;
Expand All @@ -2431,7 +2442,14 @@ describe('store', function () {
find = sinon.stub().rejects(new MongoServerError('Nope'));

try {
await fetchDocuments(dataService, '3.0.0', true, 'test.test', {});
await fetchDocuments(
dataService,
track,
'3.0.0',
true,
'test.test',
{}
);
expect.fail('Expected fetchDocuments to fail with error');
} catch (err) {
expect(find).to.have.been.calledOnce;
Expand Down
26 changes: 22 additions & 4 deletions packages/compass-crud/src/stores/crud-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,13 @@ const INITIAL_BULK_UPDATE_TEXT = `{

export const fetchDocuments: (
dataService: DataService,
track: TrackFunction,
serverVersion: string,
isDataLake: boolean,
...args: Parameters<DataService['find']>
) => Promise<HadronDocument[]> = async (
dataService: DataService,
track: TrackFunction,
serverVersion,
isDataLake,
ns,
Expand Down Expand Up @@ -145,17 +147,31 @@ export const fetchDocuments: (
};

try {
return (
let uuidSubtype3Count = 0;
let uuidSubtype4Count = 0;
const docs = (
await dataService.find(ns, filter, modifiedOptions, executionOptions)
).map((doc) => {
const { __doc, __size, ...rest } = doc;
let hadronDoc: HadronDocument;
if (__doc && __size && Object.keys(rest).length === 0) {
const hadronDoc = new HadronDocument(__doc);
hadronDoc = new HadronDocument(__doc);
hadronDoc.size = Number(__size);
return hadronDoc;
} else {
hadronDoc = new HadronDocument(doc);
}
return new HadronDocument(doc);
const { subtype3Count, subtype4Count } = hadronDoc.findUUIDs();
uuidSubtype3Count += subtype3Count;
uuidSubtype4Count += subtype4Count;
return hadronDoc;
});
if (uuidSubtype3Count > 0) {
track('UUID Encountered', { subtype: 3, count: uuidSubtype3Count });
}
if (uuidSubtype4Count > 0) {
track('UUID Encountered', { subtype: 4, count: uuidSubtype4Count });
}
Copy link
Collaborator

@addaleax addaleax May 22, 2025

Choose a reason for hiding this comment

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

Could/should these be a single event that submits two different counts?

Copy link
Collaborator Author

@paula-stacho paula-stacho May 22, 2025

Choose a reason for hiding this comment

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

I can ask analytics what is easier for them

Copy link
Collaborator Author

@paula-stacho paula-stacho May 28, 2025

Choose a reason for hiding this comment

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

looks like they have no preference. I don't expect many collections to have both types so I don't think this will cause much overload in terms of event count, so I'm merging like this.

return docs;
} catch (err) {
// We are handling all the cases where the size calculating projection might
// not work, but just in case we run into some other environment or use-case
Expand Down Expand Up @@ -896,6 +912,7 @@ class CrudStoreImpl
try {
documents = await fetchDocuments(
this.dataService,
this.track,
this.state.version,
this.state.isDataLake,
ns,
Expand Down Expand Up @@ -1733,6 +1750,7 @@ class CrudStoreImpl
),
fetchDocuments(
this.dataService,
this.track,
this.state.version,
this.state.isDataLake,
ns,
Expand Down
11 changes: 10 additions & 1 deletion packages/compass-telemetry/src/telemetry-events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2694,6 +2694,14 @@ type CreateIndexButtonClickedEvent = CommonEvent<{
};
}>;

type UUIDEncounteredEvent = CommonEvent<{
name: 'UUID Encountered';
payload: {
subtype: 3 | 4;
count: number;
};
}>;

export type TelemetryEvent =
| AggregationCanceledEvent
| AggregationCopiedEvent
Expand Down Expand Up @@ -2816,4 +2824,5 @@ export type TelemetryEvent =
| CumulativeLayoutShiftEvent
| TimeToFirstByteEvent
| ExperimentViewedEvent
| CreateIndexButtonClickedEvent;
| CreateIndexButtonClickedEvent
| UUIDEncounteredEvent;
26 changes: 25 additions & 1 deletion packages/hadron-document/src/document.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { BSONArray, BSONObject, BSONValue } from './utils';
import { objectToIdiomaticEJSON } from './utils';
import type { HadronEJSONOptions } from './utils';
import { DocumentEvents } from '.';
import type { MongoServerError } from 'mongodb';
import type { Binary, MongoServerError } from 'mongodb';

/**
* The event constant.
Expand Down Expand Up @@ -450,6 +450,30 @@ export class Document extends EventEmitter {
}, 0);
}

findUUIDs() {
let subtype4Count = 0;
let subtype3Count = 0;
for (const element of this.elements) {
if (element.currentType === 'Binary') {
if ((element.value as Binary).sub_type === 4) {
subtype4Count++;
}
if ((element.value as Binary).sub_type === 3) {
subtype3Count++;
}
} else if (
element.currentType === 'Object' ||
element.currentType === 'Array'
) {
const { subtype3Count: sub3, subtype4Count: sub4 } =
element.findUUIDs();
subtype3Count += sub3;
subtype4Count += sub4;
}
}
return { subtype3Count, subtype4Count };
}

startEditing(elementId?: string, field?: 'key' | 'value' | 'type'): void {
if (!this.editing) {
this.editing = true;
Expand Down
31 changes: 30 additions & 1 deletion packages/hadron-document/src/element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import DateEditor from './editor/date';
import Events from './element-events';
import type Document from './document';
import type { TypeCastTypes } from 'hadron-type-checker';
import type { ObjectId } from 'bson';
import type { Binary, ObjectId } from 'bson';
import type { BSONArray, BSONObject, BSONValue } from './utils';
import { getDefaultValueForType } from './utils';
import { DocumentEvents, ElementEvents } from '.';
Expand Down Expand Up @@ -879,6 +879,35 @@ export class Element extends EventEmitter {
);
}

findUUIDs(): { subtype4Count: number; subtype3Count: number } {
let subtype4Count = 0;
let subtype3Count = 0;

if (!this.elements) {
return { subtype4Count, subtype3Count };
}
for (const element of this.elements) {
if (element.currentType === 'Binary') {
if ((element.currentValue as Binary).sub_type === 4) {
subtype4Count++;
}
if ((element.currentValue as Binary).sub_type === 3) {
subtype3Count++;
}
} else if (
element.currentType === 'Object' ||
element.currentType === 'Array'
) {
const { subtype3Count: sub3, subtype4Count: sub4 } =
element.findUUIDs();
subtype3Count += sub3;
subtype4Count += sub4;
}
}

return { subtype4Count, subtype3Count };
}

private emitVisibleElementsChanged(targetElement: Element | Document = this) {
if (targetElement.isRoot()) {
targetElement.emit(DocumentEvents.VisibleElementsChanged, targetElement);
Expand Down
Loading