Skip to content

Commit a06a910

Browse files
aaron-steinfeldaaron-steinfeld
authored
Explorer groupby subpath support (#1369)
* feat: add support for attribute expressions * test: update tests * feat: add support for filtering with attribute expressions * fix: caught a couple bugs while updating tests * feat: explorer group by subpath support * test: update tests
1 parent 77c79e5 commit a06a910

File tree

10 files changed

+267
-86
lines changed

10 files changed

+267
-86
lines changed

‎projects/observability/src/pages/explorer/explorer.component.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -358,13 +358,13 @@ describe('Explorer component', () => {
358358
spectator.click(spectator.queryAll('ht-toggle-item')[1]);
359359
spectator.query(ExploreQueryEditorComponent)!.setSeries([buildSeries('second', MetricAggregationType.Average)]);
360360
spectator.query(ExploreQueryEditorComponent)!.setInterval(new TimeDuration(30, TimeUnit.Second));
361-
spectator.query(ExploreQueryEditorComponent)!.updateGroupByKey(
361+
spectator.query(ExploreQueryEditorComponent)!.updateGroupByExpression(
362362
{
363363
keyExpressions: [{ key: 'apiName' }],
364364
limit: 6,
365365
includeRest: true
366366
},
367-
'apiName'
367+
{ key: 'apiName' }
368368
);
369369
detectQueryChange();
370370
expect(queryParamChangeSpy).toHaveBeenLastCalledWith({
@@ -394,7 +394,7 @@ describe('Explorer component', () => {
394394
}
395395
});
396396
expect(spectator.query(ToggleGroupComponent)?.activeItem?.label).toBe('Spans');
397-
expect(spectator.query(ExploreQueryGroupByEditorComponent)?.groupByKey).toBe('apiName');
397+
expect(spectator.query(ExploreQueryGroupByEditorComponent)?.groupByExpression).toEqual({ key: 'apiName' });
398398
expect(spectator.query(ExploreQueryLimitEditorComponent)?.limit).toBe(6);
399399
expect(spectator.query(ExploreQueryLimitEditorComponent)?.includeRest).toBe(true);
400400
expect(spectator.query(ExploreQuerySeriesEditorComponent)?.series).toEqual({

‎projects/observability/src/pages/explorer/explorer.component.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ export class ExplorerComponent {
308308
private tryDecodeAttributeExpression(expressionString: string): [AttributeExpression] | [] {
309309
const [key, subpath] = expressionString.split('__');
310310

311-
return [{ key: key, ...(isEmpty(subpath) ? { subpath: subpath } : {}) }];
311+
return [{ key: key, ...(!isEmpty(subpath) ? { subpath: subpath } : {}) }];
312312
}
313313
}
314314
interface ContextToggleItem extends ToggleItem<ExplorerContextScope> {

‎projects/observability/src/shared/components/explore-query-editor/explore-query-editor.component.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ describe('Explore query editor', () => {
170170
const options = spectator.queryAll('.select-option', { root: true });
171171
spectator.click(options[1]);
172172

173-
spectator.tick(10);
173+
spectator.tick(510); // Debounced
174174

175175
expect(onRequestChange).toHaveBeenLastCalledWith(
176176
expect.objectContaining({
@@ -204,7 +204,7 @@ describe('Explore query editor', () => {
204204
const options = spectator.queryAll('.select-option', { root: true });
205205
spectator.click(options[1]);
206206

207-
spectator.tick(10);
207+
spectator.tick(510);
208208

209209
const limitInputEl = spectator.query('ht-explore-query-limit-editor input') as HTMLInputElement;
210210
limitInputEl.value = '6';
@@ -271,7 +271,7 @@ describe('Explore query editor', () => {
271271
const options = spectator.queryAll('.select-option', { root: true });
272272
spectator.click(options[1]);
273273

274-
spectator.tick(10);
274+
spectator.tick(510); // Debounced
275275

276276
spectator.click('.limit-include-rest-container input[type="checkbox"]');
277277

‎projects/observability/src/shared/components/explore-query-editor/explore-query-editor.component.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnChanges, OnI
22
import { TypedSimpleChanges } from '@hypertrace/common';
33
import { Filter } from '@hypertrace/components';
44
import { Observable } from 'rxjs';
5+
import { AttributeExpression } from '../../graphql/model/attribute/attribute-expression';
56
import { GraphQlGroupBy } from '../../graphql/model/schema/groupby/graphql-group-by';
67
import { IntervalValue } from '../interval-select/interval-select.component';
78
import {
@@ -39,8 +40,8 @@ import {
3940
<ht-explore-query-group-by-editor
4041
class="group-by"
4142
[context]="currentVisualization.context"
42-
[groupByKey]="(currentVisualization.groupBy?.keyExpressions)?.[0]?.key"
43-
(groupByKeyChange)="this.updateGroupByKey(currentVisualization.groupBy, $event)"
43+
[groupByExpression]="(currentVisualization.groupBy?.keyExpressions)[0]"
44+
(groupByExpressionChange)="this.updateGroupByExpression(currentVisualization.groupBy, $event)"
4445
></ht-explore-query-group-by-editor>
4546
4647
<ht-explore-query-limit-editor
@@ -104,22 +105,22 @@ export class ExploreQueryEditorComponent implements OnChanges, OnInit {
104105
}
105106

106107
if (changeObject.groupBy && this.groupBy?.keyExpressions.length) {
107-
this.updateGroupByKey(this.groupBy, this.groupBy.keyExpressions[0]?.key);
108+
this.updateGroupByExpression(this.groupBy, this.groupBy.keyExpressions[0]);
108109
}
109110
}
110111

111112
public setSeries(series: ExploreSeries[]): void {
112113
this.visualizationBuilder.setSeries(series);
113114
}
114115

115-
public updateGroupByKey(groupBy?: GraphQlGroupBy, key?: string): void {
116-
if (key === undefined) {
116+
public updateGroupByExpression(groupBy?: GraphQlGroupBy, keyExpression?: AttributeExpression): void {
117+
if (keyExpression === undefined) {
117118
this.visualizationBuilder.groupBy();
118119
} else {
119120
this.visualizationBuilder.groupBy(
120121
groupBy
121-
? { ...groupBy, keyExpressions: [{ key: key }] }
122-
: { keyExpressions: [{ key: key }], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT }
122+
? { ...groupBy, keyExpressions: [keyExpression] }
123+
: { keyExpressions: [keyExpression], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT }
123124
);
124125
}
125126
}

‎projects/observability/src/shared/components/explore-query-editor/explore-query-editor.module.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
import { CommonModule } from '@angular/common';
22
import { NgModule } from '@angular/core';
3-
import { ButtonModule, InputModule, SelectModule, TooltipModule, TraceCheckboxModule } from '@hypertrace/components';
3+
import {
4+
ButtonModule,
5+
FormFieldModule,
6+
InputModule,
7+
LetAsyncModule,
8+
SelectModule,
9+
TooltipModule,
10+
TraceCheckboxModule
11+
} from '@hypertrace/components';
412
import { IntervalSelectModule } from '../interval-select/interval-select.module';
513
import { ExploreQueryEditorComponent } from './explore-query-editor.component';
614
import { ExploreQueryGroupByEditorComponent } from './group-by/explore-query-group-by-editor.component';
@@ -26,7 +34,9 @@ import { ExploreQuerySeriesGroupEditorComponent } from './series/explore-query-s
2634
TooltipModule,
2735
InputModule,
2836
IntervalSelectModule,
29-
TraceCheckboxModule
37+
TraceCheckboxModule,
38+
LetAsyncModule,
39+
FormFieldModule
3040
]
3141
})
3242
export class ExploreQueryEditorModule {}

‎projects/observability/src/shared/components/explore-query-editor/group-by/explore-query-group-by-editor.component.scss‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,29 @@
33

44
.group-by-container {
55
display: flex;
6-
flex-direction: column;
6+
flex-direction: row;
7+
gap: 24px;
78

8-
.group-by-label {
9-
@include body-1-medium($gray-9);
10-
height: 32px;
11-
line-height: 32px;
12-
margin-bottom: 12px;
9+
.group-by-input-container {
10+
display: flex;
11+
flex-direction: column;
12+
13+
.group-by-label {
14+
@include body-1-medium($gray-9);
15+
height: 32px;
16+
line-height: 32px;
17+
margin-bottom: 12px;
18+
}
19+
20+
.group-by-path-wrapper {
21+
width: 100px;
22+
23+
.group-by-path-input {
24+
@include body-2-regular($gray-9);
25+
width: 100%;
26+
height: 100%;
27+
line-height: 32px;
28+
}
29+
}
1330
}
1431
}

‎projects/observability/src/shared/components/explore-query-editor/group-by/explore-query-group-by-editor.component.test.ts‎

Lines changed: 111 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,24 @@ import { HttpClientTestingModule } from '@angular/common/http/testing';
22
import { fakeAsync, flush } from '@angular/core/testing';
33
import { IconLibraryTestingModule } from '@hypertrace/assets-library';
44
import { NavigationService } from '@hypertrace/common';
5-
import { SelectComponent, SelectModule } from '@hypertrace/components';
5+
import { InputModule, LetAsyncModule, SelectComponent, SelectModule } from '@hypertrace/components';
66
import { byText, createHostFactory, mockProvider } from '@ngneat/spectator/jest';
77
import { EMPTY, of } from 'rxjs';
8-
import { AttributeMetadata } from '../../../graphql/model/metadata/attribute-metadata';
8+
import { AttributeMetadata, AttributeMetadataType } from '../../../graphql/model/metadata/attribute-metadata';
99
import { ObservabilityTraceType } from '../../../graphql/model/schema/observability-traces';
1010
import { MetadataService } from '../../../services/metadata/metadata.service';
1111
import { ExploreQueryGroupByEditorComponent } from './explore-query-group-by-editor.component';
1212

1313
describe('Explore Query Group by Editor component', () => {
1414
// Define metadata at top level so equality checks always get same values
15-
const attributeMetadata = [{ name: 'test value' }, { name: 'foo bar' }];
15+
const attributeMetadata = [
16+
{ name: 'test value', type: AttributeMetadataType.String },
17+
{ name: 'foo bar', type: AttributeMetadataType.String },
18+
{ name: 'test map', type: AttributeMetadataType.StringMap }
19+
];
1620
const hostBuilder = createHostFactory({
1721
component: ExploreQueryGroupByEditorComponent,
18-
imports: [SelectModule, HttpClientTestingModule, IconLibraryTestingModule],
22+
imports: [SelectModule, HttpClientTestingModule, IconLibraryTestingModule, LetAsyncModule, InputModule],
1923
providers: [
2024
mockProvider(MetadataService, {
2125
getAttributeDisplayName: (attribute: AttributeMetadata) => attribute.name,
@@ -29,13 +33,11 @@ describe('Explore Query Group by Editor component', () => {
2933
});
3034

3135
test('sets group by to none if undefined provided', fakeAsync(() => {
32-
const spectator = hostBuilder(
33-
`
36+
const spectator = hostBuilder(`
3437
<ht-explore-query-group-by-editor
35-
context="${ObservabilityTraceType.Api}" [groupByKey]="groupBy"
38+
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
3639
></ht-explore-query-group-by-editor>
37-
`
38-
);
40+
`);
3941
spectator.tick();
4042

4143
expect(spectator.query(SelectComponent)!.selected).toBeUndefined();
@@ -46,38 +48,38 @@ describe('Explore Query Group by Editor component', () => {
4648
const spectator = hostBuilder(
4749
`
4850
<ht-explore-query-group-by-editor
49-
context="${ObservabilityTraceType.Api}" [groupByKey]="groupByKey"
51+
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
5052
></ht-explore-query-group-by-editor>
5153
`,
5254
{
5355
hostProps: {
54-
groupByKey: 'test value'
56+
groupByExpression: { key: 'test value' }
5557
}
5658
}
5759
);
5860

59-
expect(spectator.query(SelectComponent)!.selected).toBe('test value');
61+
expect(spectator.query(SelectComponent)!.selected).toEqual(attributeMetadata[0]);
6062
});
6163

6264
test('updates if new group by is provided', () => {
6365
const spectator = hostBuilder(
6466
`
6567
<ht-explore-query-group-by-editor
66-
context="${ObservabilityTraceType.Api}" [groupByKey]="groupByKey"
68+
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
6769
></ht-explore-query-group-by-editor>
6870
`,
6971
{
7072
hostProps: {
71-
groupByKey: 'test value'
73+
groupByExpression: { key: 'test value' }
7274
}
7375
}
7476
);
7577

76-
spectator.setHostInput({ groupByKey: 'foo bar' });
78+
spectator.setHostInput({ groupByExpression: { key: 'foo bar' } });
7779

78-
expect(spectator.query(SelectComponent)!.selected).toBe('foo bar');
80+
expect(spectator.query(SelectComponent)!.selected).toEqual(attributeMetadata[1]);
7981

80-
spectator.setHostInput({ groupByKey: undefined });
82+
spectator.setHostInput({ groupByExpression: undefined });
8183

8284
expect(spectator.query(SelectComponent)!.selected).toBeUndefined();
8385
});
@@ -87,12 +89,42 @@ describe('Explore Query Group by Editor component', () => {
8789
const spectator = hostBuilder(
8890
`
8991
<ht-explore-query-group-by-editor
90-
context="${ObservabilityTraceType.Api}" [groupByKey]="groupByKey" (groupByKeyChange)="onChange($event)"
92+
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression" (groupByExpressionChange)="onChange($event)"
93+
></ht-explore-query-group-by-editor>
94+
`,
95+
{
96+
hostProps: {
97+
groupByExpression: { key: 'test value' },
98+
onChange: onChange
99+
}
100+
}
101+
);
102+
spectator.tick(); // Needs to tick to interact with select which is async
103+
104+
spectator.click(spectator.query(byText('test value'))!);
105+
106+
const options = spectator.queryAll('.select-option', { root: true });
107+
expect(options.length).toBe(4);
108+
spectator.click(options[2]);
109+
110+
spectator.tick(500); // 500ms debounce after group by change
111+
expect(onChange).toHaveBeenCalledTimes(1);
112+
expect(onChange).toHaveBeenCalledWith({ key: 'foo bar' });
113+
114+
flush(); // Overlay removes async
115+
}));
116+
117+
test('emits when group by key is changed', fakeAsync(() => {
118+
const onChange = jest.fn();
119+
const spectator = hostBuilder(
120+
`
121+
<ht-explore-query-group-by-editor
122+
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression" (groupByExpressionChange)="onChange($event)"
91123
></ht-explore-query-group-by-editor>
92124
`,
93125
{
94126
hostProps: {
95-
groupByKey: 'test value',
127+
groupByExpression: { key: 'test value' },
96128
onChange: onChange
97129
}
98130
}
@@ -102,12 +134,70 @@ describe('Explore Query Group by Editor component', () => {
102134
spectator.click(spectator.query(byText('test value'))!);
103135

104136
const options = spectator.queryAll('.select-option', { root: true });
105-
expect(options.length).toBe(3);
137+
expect(options.length).toBe(4);
106138
spectator.click(options[2]);
107139

140+
spectator.tick(500); // 500ms debounce after group by change
108141
expect(onChange).toHaveBeenCalledTimes(1);
109-
expect(onChange).toHaveBeenCalledWith('foo bar');
142+
expect(onChange).toHaveBeenCalledWith({ key: 'foo bar' });
143+
144+
flush(); // Overlay removes async
145+
}));
146+
147+
test('shows subpath for map attributes only', fakeAsync(() => {
148+
const spectator = hostBuilder(
149+
`
150+
<ht-explore-query-group-by-editor
151+
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
152+
></ht-explore-query-group-by-editor>
153+
`,
154+
{
155+
hostProps: {
156+
groupByExpression: { key: 'test value' }
157+
}
158+
}
159+
);
160+
spectator.tick(); // Needs to tick to interact with select which is async
161+
162+
expect(spectator.query('.select')).toHaveText('test value');
163+
expect(spectator.query('.group-by-path-input')).not.toExist();
164+
spectator.click(spectator.query(byText('test value'))!);
165+
const options = spectator.queryAll('.select-option', { root: true });
166+
spectator.click(options[3]);
167+
168+
expect(spectator.query('.group-by-path-input')).toExist();
169+
170+
flush(); // Overlay removes async
171+
}));
110172

173+
test('outputs map expressions correctly', fakeAsync(() => {
174+
const onChange = jest.fn();
175+
const spectator = hostBuilder(
176+
`
177+
<ht-explore-query-group-by-editor
178+
context="${ObservabilityTraceType.Api}" (groupByExpressionChange)="onChange($event)"
179+
></ht-explore-query-group-by-editor>
180+
`,
181+
{
182+
hostProps: {
183+
onChange: onChange
184+
}
185+
}
186+
);
187+
spectator.tick(); // Needs to tick to interact with select which is async
188+
expect(onChange).not.toHaveBeenCalled();
189+
spectator.click(spectator.query(byText('None'))!);
190+
const options = spectator.queryAll('.select-option', { root: true });
191+
spectator.click(options[3]);
192+
// Wait for debounce
193+
spectator.tick(500);
194+
// Should not emit on switching to map group by, not eligible without a subpath
195+
expect(onChange).not.toHaveBeenCalled();
196+
spectator.typeInElement('test.subpath', '.group-by-path-input input');
197+
expect(onChange).not.toHaveBeenCalled();
198+
// Wait for debounce
199+
spectator.tick(500);
200+
expect(onChange).toHaveBeenCalledWith({ key: 'test map', subpath: 'test.subpath' });
111201
flush(); // Overlay removes async
112202
}));
113203
});

0 commit comments

Comments
 (0)