Skip to content

Explorer groupby subpath support #1369

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.

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 10 commits into from
Jan 13, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -358,13 +358,13 @@ describe('Explorer component', () => {
spectator.click(spectator.queryAll('ht-toggle-item')[1]);
spectator.query(ExploreQueryEditorComponent)!.setSeries([buildSeries('second', MetricAggregationType.Average)]);
spectator.query(ExploreQueryEditorComponent)!.setInterval(new TimeDuration(30, TimeUnit.Second));
spectator.query(ExploreQueryEditorComponent)!.updateGroupByKey(
spectator.query(ExploreQueryEditorComponent)!.updateGroupByExpression(
{
keyExpressions: [{ key: 'apiName' }],
limit: 6,
includeRest: true
},
'apiName'
{ key: 'apiName' }
);
detectQueryChange();
expect(queryParamChangeSpy).toHaveBeenLastCalledWith({
Expand Down Expand Up @@ -394,7 +394,7 @@ describe('Explorer component', () => {
}
});
expect(spectator.query(ToggleGroupComponent)?.activeItem?.label).toBe('Spans');
expect(spectator.query(ExploreQueryGroupByEditorComponent)?.groupByKey).toBe('apiName');
expect(spectator.query(ExploreQueryGroupByEditorComponent)?.groupByExpression).toEqual({ key: 'apiName' });
expect(spectator.query(ExploreQueryLimitEditorComponent)?.limit).toBe(6);
expect(spectator.query(ExploreQueryLimitEditorComponent)?.includeRest).toBe(true);
expect(spectator.query(ExploreQuerySeriesEditorComponent)?.series).toEqual({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ export class ExplorerComponent {
private tryDecodeAttributeExpression(expressionString: string): [AttributeExpression] | [] {
const [key, subpath] = expressionString.split('__');

return [{ key: key, ...(isEmpty(subpath) ? { subpath: subpath } : {}) }];
return [{ key: key, ...(!isEmpty(subpath) ? { subpath: subpath } : {}) }];
}
}
interface ContextToggleItem extends ToggleItem<ExplorerContextScope> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ describe('Explore query editor', () => {
const options = spectator.queryAll('.select-option', { root: true });
spectator.click(options[1]);

spectator.tick(10);
spectator.tick(510); // Debounced

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

spectator.tick(10);
spectator.tick(510);

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

spectator.tick(10);
spectator.tick(510); // Debounced

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnChanges, OnI
import { TypedSimpleChanges } from '@hypertrace/common';
import { Filter } from '@hypertrace/components';
import { Observable } from 'rxjs';
import { AttributeExpression } from '../../graphql/model/attribute/attribute-expression';
import { GraphQlGroupBy } from '../../graphql/model/schema/groupby/graphql-group-by';
import { IntervalValue } from '../interval-select/interval-select.component';
import {
Expand Down Expand Up @@ -39,8 +40,8 @@ import {
<ht-explore-query-group-by-editor
class="group-by"
[context]="currentVisualization.context"
[groupByKey]="(currentVisualization.groupBy?.keyExpressions)?.[0]?.key"
(groupByKeyChange)="this.updateGroupByKey(currentVisualization.groupBy, $event)"
[groupByExpression]="(currentVisualization.groupBy?.keyExpressions)[0]"
(groupByExpressionChange)="this.updateGroupByExpression(currentVisualization.groupBy, $event)"
></ht-explore-query-group-by-editor>

<ht-explore-query-limit-editor
Expand Down Expand Up @@ -104,22 +105,22 @@ export class ExploreQueryEditorComponent implements OnChanges, OnInit {
}

if (changeObject.groupBy && this.groupBy?.keyExpressions.length) {
this.updateGroupByKey(this.groupBy, this.groupBy.keyExpressions[0]?.key);
this.updateGroupByExpression(this.groupBy, this.groupBy.keyExpressions[0]);
}
}

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

public updateGroupByKey(groupBy?: GraphQlGroupBy, key?: string): void {
if (key === undefined) {
public updateGroupByExpression(groupBy?: GraphQlGroupBy, keyExpression?: AttributeExpression): void {
if (keyExpression === undefined) {
this.visualizationBuilder.groupBy();
} else {
this.visualizationBuilder.groupBy(
groupBy
? { ...groupBy, keyExpressions: [{ key: key }] }
: { keyExpressions: [{ key: key }], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT }
? { ...groupBy, keyExpressions: [keyExpression] }
: { keyExpressions: [keyExpression], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT }
);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';
import { ButtonModule, InputModule, SelectModule, TooltipModule, TraceCheckboxModule } from '@hypertrace/components';
import {
ButtonModule,
FormFieldModule,
InputModule,
LetAsyncModule,
SelectModule,
TooltipModule,
TraceCheckboxModule
} from '@hypertrace/components';
import { IntervalSelectModule } from '../interval-select/interval-select.module';
import { ExploreQueryEditorComponent } from './explore-query-editor.component';
import { ExploreQueryGroupByEditorComponent } from './group-by/explore-query-group-by-editor.component';
Expand All @@ -26,7 +34,9 @@ import { ExploreQuerySeriesGroupEditorComponent } from './series/explore-query-s
TooltipModule,
InputModule,
IntervalSelectModule,
TraceCheckboxModule
TraceCheckboxModule,
LetAsyncModule,
FormFieldModule
]
})
export class ExploreQueryEditorModule {}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,29 @@

.group-by-container {
display: flex;
flex-direction: column;
flex-direction: row;
gap: 24px;

.group-by-label {
@include body-1-medium($gray-9);
height: 32px;
line-height: 32px;
margin-bottom: 12px;
.group-by-input-container {
display: flex;
flex-direction: column;

.group-by-label {
@include body-1-medium($gray-9);
height: 32px;
line-height: 32px;
margin-bottom: 12px;
}

.group-by-path-wrapper {
width: 100px;

.group-by-path-input {
@include body-2-regular($gray-9);
width: 100%;
height: 100%;
line-height: 32px;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,24 @@ import { HttpClientTestingModule } from '@angular/common/http/testing';
import { fakeAsync, flush } from '@angular/core/testing';
import { IconLibraryTestingModule } from '@hypertrace/assets-library';
import { NavigationService } from '@hypertrace/common';
import { SelectComponent, SelectModule } from '@hypertrace/components';
import { InputModule, LetAsyncModule, SelectComponent, SelectModule } from '@hypertrace/components';
import { byText, createHostFactory, mockProvider } from '@ngneat/spectator/jest';
import { EMPTY, of } from 'rxjs';
import { AttributeMetadata } from '../../../graphql/model/metadata/attribute-metadata';
import { AttributeMetadata, AttributeMetadataType } from '../../../graphql/model/metadata/attribute-metadata';
import { ObservabilityTraceType } from '../../../graphql/model/schema/observability-traces';
import { MetadataService } from '../../../services/metadata/metadata.service';
import { ExploreQueryGroupByEditorComponent } from './explore-query-group-by-editor.component';

describe('Explore Query Group by Editor component', () => {
// Define metadata at top level so equality checks always get same values
const attributeMetadata = [{ name: 'test value' }, { name: 'foo bar' }];
const attributeMetadata = [
{ name: 'test value', type: AttributeMetadataType.String },
{ name: 'foo bar', type: AttributeMetadataType.String },
{ name: 'test map', type: AttributeMetadataType.StringMap }
];
const hostBuilder = createHostFactory({
component: ExploreQueryGroupByEditorComponent,
imports: [SelectModule, HttpClientTestingModule, IconLibraryTestingModule],
imports: [SelectModule, HttpClientTestingModule, IconLibraryTestingModule, LetAsyncModule, InputModule],
providers: [
mockProvider(MetadataService, {
getAttributeDisplayName: (attribute: AttributeMetadata) => attribute.name,
Expand All @@ -29,13 +33,11 @@ describe('Explore Query Group by Editor component', () => {
});

test('sets group by to none if undefined provided', fakeAsync(() => {
const spectator = hostBuilder(
`
const spectator = hostBuilder(`
<ht-explore-query-group-by-editor
context="${ObservabilityTraceType.Api}" [groupByKey]="groupBy"
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
></ht-explore-query-group-by-editor>
`
);
`);
spectator.tick();

expect(spectator.query(SelectComponent)!.selected).toBeUndefined();
Expand All @@ -46,38 +48,38 @@ describe('Explore Query Group by Editor component', () => {
const spectator = hostBuilder(
`
<ht-explore-query-group-by-editor
context="${ObservabilityTraceType.Api}" [groupByKey]="groupByKey"
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
></ht-explore-query-group-by-editor>
`,
{
hostProps: {
groupByKey: 'test value'
groupByExpression: { key: 'test value' }
}
}
);

expect(spectator.query(SelectComponent)!.selected).toBe('test value');
expect(spectator.query(SelectComponent)!.selected).toEqual(attributeMetadata[0]);
});

test('updates if new group by is provided', () => {
const spectator = hostBuilder(
`
<ht-explore-query-group-by-editor
context="${ObservabilityTraceType.Api}" [groupByKey]="groupByKey"
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
></ht-explore-query-group-by-editor>
`,
{
hostProps: {
groupByKey: 'test value'
groupByExpression: { key: 'test value' }
}
}
);

spectator.setHostInput({ groupByKey: 'foo bar' });
spectator.setHostInput({ groupByExpression: { key: 'foo bar' } });

expect(spectator.query(SelectComponent)!.selected).toBe('foo bar');
expect(spectator.query(SelectComponent)!.selected).toEqual(attributeMetadata[1]);

spectator.setHostInput({ groupByKey: undefined });
spectator.setHostInput({ groupByExpression: undefined });

expect(spectator.query(SelectComponent)!.selected).toBeUndefined();
});
Expand All @@ -87,12 +89,42 @@ describe('Explore Query Group by Editor component', () => {
const spectator = hostBuilder(
`
<ht-explore-query-group-by-editor
context="${ObservabilityTraceType.Api}" [groupByKey]="groupByKey" (groupByKeyChange)="onChange($event)"
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression" (groupByExpressionChange)="onChange($event)"
></ht-explore-query-group-by-editor>
`,
{
hostProps: {
groupByExpression: { key: 'test value' },
onChange: onChange
}
}
);
spectator.tick(); // Needs to tick to interact with select which is async

spectator.click(spectator.query(byText('test value'))!);

const options = spectator.queryAll('.select-option', { root: true });
expect(options.length).toBe(4);
spectator.click(options[2]);

spectator.tick(500); // 500ms debounce after group by change
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith({ key: 'foo bar' });

flush(); // Overlay removes async
}));

test('emits when group by key is changed', fakeAsync(() => {
const onChange = jest.fn();
const spectator = hostBuilder(
`
<ht-explore-query-group-by-editor
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression" (groupByExpressionChange)="onChange($event)"
></ht-explore-query-group-by-editor>
`,
{
hostProps: {
groupByKey: 'test value',
groupByExpression: { key: 'test value' },
onChange: onChange
}
}
Expand All @@ -102,12 +134,70 @@ describe('Explore Query Group by Editor component', () => {
spectator.click(spectator.query(byText('test value'))!);

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

spectator.tick(500); // 500ms debounce after group by change
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith('foo bar');
expect(onChange).toHaveBeenCalledWith({ key: 'foo bar' });

flush(); // Overlay removes async
}));

test('shows subpath for map attributes only', fakeAsync(() => {
const spectator = hostBuilder(
`
<ht-explore-query-group-by-editor
context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
></ht-explore-query-group-by-editor>
`,
{
hostProps: {
groupByExpression: { key: 'test value' }
}
}
);
spectator.tick(); // Needs to tick to interact with select which is async

expect(spectator.query('.select')).toHaveText('test value');
expect(spectator.query('.group-by-path-input')).not.toExist();
spectator.click(spectator.query(byText('test value'))!);
const options = spectator.queryAll('.select-option', { root: true });
spectator.click(options[3]);

expect(spectator.query('.group-by-path-input')).toExist();

flush(); // Overlay removes async
}));

test('outputs map expressions correctly', fakeAsync(() => {
const onChange = jest.fn();
const spectator = hostBuilder(
`
<ht-explore-query-group-by-editor
context="${ObservabilityTraceType.Api}" (groupByExpressionChange)="onChange($event)"
></ht-explore-query-group-by-editor>
`,
{
hostProps: {
onChange: onChange
}
}
);
spectator.tick(); // Needs to tick to interact with select which is async
expect(onChange).not.toHaveBeenCalled();
spectator.click(spectator.query(byText('None'))!);
const options = spectator.queryAll('.select-option', { root: true });
spectator.click(options[3]);
// Wait for debounce
spectator.tick(500);
// Should not emit on switching to map group by, not eligible without a subpath
expect(onChange).not.toHaveBeenCalled();
spectator.typeInElement('test.subpath', '.group-by-path-input input');
expect(onChange).not.toHaveBeenCalled();
// Wait for debounce
spectator.tick(500);
expect(onChange).toHaveBeenCalledWith({ key: 'test map', subpath: 'test.subpath' });
flush(); // Overlay removes async
}));
});
Loading