Skip to content

feat: using telemetry tracker in few atomic components #1129

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 21 commits into from
Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
99388e6
feat: adding user telemetry config and service
anandtiwary Sep 15, 2021
76ef6f3
feat: adding error handler and a track directive
anandtiwary Sep 15, 2021
a93db20
feat: adding ootb providers
anandtiwary Sep 15, 2021
8a4b536
feat: using telemetry in with link components
anandtiwary Sep 15, 2021
aa91d22
Merge branch 'main' into user-telemetry-2
anandtiwary Sep 21, 2021
6161655
refactor: using abstract service approach
anandtiwary Sep 21, 2021
a8b308d
refactor: updating api for htTrack directive
anandtiwary Sep 22, 2021
9ce27b3
revert: package-lock.json
anandtiwary Sep 22, 2021
1358760
revert: revert package.json
anandtiwary Sep 22, 2021
8395555
revert: revert package.json peer deps
anandtiwary Sep 22, 2021
4db25a3
refactor: adding tests and addressing commments
anandtiwary Sep 24, 2021
8abcc0a
refactor: fix lint errors
anandtiwary Sep 25, 2021
ab55e02
Merge branch 'user-telemetry-2' into user-telemetry-3
anandtiwary Sep 26, 2021
816baec
refactor: adding packages and modifying code
anandtiwary Sep 26, 2021
771c129
Merge branch 'user-telemetry-3' into user-telemetry-4
anandtiwary Sep 26, 2021
226edaa
refactor: adding tracking directive on common components
anandtiwary Sep 26, 2021
f27a899
Merge branch 'main' into user-telemetry-4
anandtiwary Sep 27, 2021
0da628a
refactor: fixing tests
anandtiwary Sep 28, 2021
f82244d
Merge branch 'main' into user-telemetry-4
anandtiwary Sep 28, 2021
aef030a
revert: package json changes
anandtiwary Sep 28, 2021
b187f00
refactor: adding mixpanel types
anandtiwary Sep 28, 2021
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
13 changes: 13 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"@angular/platform-browser-dynamic": "^12.2.1",
"@angular/router": "^12.2.1",
"@apollo/client": "^3.4.13",
"@fullstory/browser": "^1.4.9",
"@hypertrace/hyperdash": "^1.2.1",
"@hypertrace/hyperdash-angular": "^2.6.0",
"@types/d3-hierarchy": "^2.0.0",
Expand All @@ -62,12 +63,11 @@
"graphql-tag": "^2.12.5",
"iso8601-duration": "^1.3.0",
"lodash-es": "^4.17.21",
"mixpanel-browser": "^2.41.0",
"rxjs": "~6.6.7",
"tslib": "^2.3.1",
"uuid": "^8.3.2",
"zone.js": "~0.11.4",
"@fullstory/browser": "^1.4.9",
"mixpanel-browser": "^2.41.0"
"zone.js": "~0.11.4"
},
"devDependencies": {
"@angular-builders/jest": "^11.2.0",
Expand All @@ -92,6 +92,7 @@
"@types/d3-zoom": "^1.7.5",
"@types/jest": "^26.0.24",
"@types/lodash-es": "^4.17.4",
"@types/mixpanel-browser": "^2.35.7",
"@types/node": "^16.7.10",
"@types/uuid": "^8.3.1",
"@types/webpack-env": "^1.16.2",
Expand Down
11 changes: 11 additions & 0 deletions projects/common/src/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,17 @@ export * from './utilities/rxjs/subscription-lifeycle.service';
export * from './utilities/types/angular-change-object';
export * from './utilities/types/types';

// Telemetry
export * from './telemetry/user-telemetry.module';
export * from './telemetry/track/user-telemetry-tracking.module';
export * from './telemetry/track/track.directive';
export * from './telemetry/user-telemetry.service';
export * from './telemetry/telemetry';
export { FullStoryTelemetry } from './telemetry/providers/fullstory/full-story-provider';
export { FreshPaintTelemetry } from './telemetry/providers/freshpaint/freshpaint-provider';
export { MixPanelTelemetry } from './telemetry/providers/mixpanel/mixpanel-provider';
export { TrackDirective } from './telemetry/track/track.directive';

// Time
export * from './time/fixed-time-range';
export * from './time/interval-duration.service';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// tslint:disable

export const loadFreshPaint = () => {
const loadFreshPaint = () => {
if (window.freshpaint) {
return window.freshpaint;
}
Expand Down Expand Up @@ -79,3 +79,7 @@ export const loadFreshPaint = () => {

return freshpaint;
};

module.exports = {
loadFreshPaint
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// tslint:disable

export const loadGA = () => {
const loadGA = () => {
/**
* Creates a temporary global ga object and loads analytics.js.
* Parameters o, a, and m are all used internally. They could have been
Expand Down Expand Up @@ -49,3 +49,7 @@ export const loadGA = () => {

return ga;
};

module.exports = {
loadGA
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';
import { TrackDirective } from './track.directive';

@NgModule({
imports: [CommonModule],
declarations: [TrackDirective],
exports: [TrackDirective]
})
export class UserTelemetryTrackingModule {}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Injectable, Injector } from '@angular/core';
import { Injectable, Injector, Optional } from '@angular/core';
import { NavigationEnd, Router } from '@angular/router';
import { filter } from 'rxjs/operators';
import { Dictionary } from '../utilities/types/types';
Expand All @@ -9,7 +9,7 @@ import { UserTelemetryService } from './user-telemetry.service';
export class UserTelemetryImplService extends UserTelemetryService {
private telemetryProviders: UserTelemetryInternalConfig[] = [];

public constructor(private readonly injector: Injector, private readonly router: Router) {
public constructor(private readonly injector: Injector, @Optional() private readonly router?: Router) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, if I don't make Router optional then I get test errors wherever the atomic component (that uses htTrack) is used. Alternatively, I will have to provide Router Mock service everywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

But shouldn't we be mocking the component in those tests? Why are they use the real track component?

super();
this.setupAutomaticPageTracking();
}
Expand Down Expand Up @@ -69,7 +69,7 @@ export class UserTelemetryImplService extends UserTelemetryService {
}

private setupAutomaticPageTracking(): void {
this.router.events
this.router?.events
.pipe(filter((event): event is NavigationEnd => event instanceof NavigationEnd))
.subscribe(route => this.trackPageEvent(`Visited: ${route.url}`, { url: route.url }));
}
Expand Down
9 changes: 5 additions & 4 deletions projects/common/src/telemetry/user-telemetry.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import { UserTelemetryRegistrationConfig } from './telemetry';
import { UserTelemetryImplService } from './user-telemetry-impl.service';
import { UserTelemetryService } from './user-telemetry.service';

const USER_TELEMETRY_PROVIDER_TOKENS = new InjectionToken<UserTelemetryRegistrationConfig<unknown>[][]>(
'USER_TELEMETRY_PROVIDER_TOKENS'
);

@NgModule()
// tslint:disable:no-unnecessary-class
export class UserTelemetryModule {
public constructor(
@Inject(USER_TELEMETRY_PROVIDER_TOKENS) providerConfigs: UserTelemetryRegistrationConfig<unknown>[][],
Expand Down Expand Up @@ -35,7 +40,3 @@ export class UserTelemetryModule {
};
}
}

const USER_TELEMETRY_PROVIDER_TOKENS = new InjectionToken<UserTelemetryRegistrationConfig<unknown>[][]>(
'USER_TELEMETRY_PROVIDER_TOKENS'
);
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { HttpClientTestingModule } from '@angular/common/http/testing';
import { RouterTestingModule } from '@angular/router/testing';
import { IconLibraryTestingModule } from '@hypertrace/assets-library';
import { NavigationService } from '@hypertrace/common';
import { NavigationService, TrackDirective } from '@hypertrace/common';
import { byText, createHostFactory, mockProvider, Spectator } from '@ngneat/spectator/jest';
import { MockDirective } from 'ng-mocks';
import { EMPTY } from 'rxjs';
import { BreadcrumbsComponent } from './breadcrumbs.component';
import { BreadcrumbsModule } from './breadcrumbs.module';
Expand All @@ -13,7 +15,8 @@ describe('BreadcrumbsComponent', () => {
const createHost = createHostFactory({
declareComponent: false,
component: BreadcrumbsComponent,
imports: [BreadcrumbsModule, HttpClientTestingModule, IconLibraryTestingModule],
imports: [BreadcrumbsModule, HttpClientTestingModule, IconLibraryTestingModule, RouterTestingModule],
declarations: [MockDirective(TrackDirective)],
Copy link
Contributor

Choose a reason for hiding this comment

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

So here's an example. The mocked track component should be sufficient, we shouldn't need the router testing module too (and in general, this test needs to be converted be shallow)

Copy link
Contributor Author

@anandtiwary anandtiwary Sep 28, 2021

Choose a reason for hiding this comment

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

The MockDirective still tries to inject Router since it is in the constructor.

Copy link
Contributor

Choose a reason for hiding this comment

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

really? that seems off...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, they try to inject the services. I have seen it at few other places.

providers: [
mockProvider(NavigationService, {
navigation$: EMPTY,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { IconSize } from '../icon/icon-size';
[ngClass]="{ navigable: breadcrumb.url !== undefined }"
*ngFor="let breadcrumb of this.breadcrumbs; last as isLast; first as isFirst"
[htTooltip]="this.tooltipMap.get(breadcrumb)"
[htTrack]
[htTrackLabel]="breadcrumb.label"
>
<ht-icon
class="icon"
Expand Down
3 changes: 2 additions & 1 deletion projects/components/src/breadcrumbs/breadcrumbs.module.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';
import { UserTelemetryTrackingModule } from '@hypertrace/common';
import { IconModule } from '../icon/icon.module';
import { LabelModule } from '../label/label.module';
import { TooltipModule } from '../tooltip/tooltip.module';
import { BreadcrumbsComponent } from './breadcrumbs.component';

@NgModule({
imports: [CommonModule, IconModule, LabelModule, TooltipModule],
imports: [CommonModule, IconModule, LabelModule, TooltipModule, UserTelemetryTrackingModule],
declarations: [BreadcrumbsComponent],
exports: [BreadcrumbsComponent]
})
Expand Down
3 changes: 3 additions & 0 deletions projects/components/src/button/button.component.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { RouterTestingModule } from '@angular/router/testing';
import { IconType } from '@hypertrace/assets-library';
import { TrackDirective } from '@hypertrace/common';
import { createHostFactory, Spectator } from '@ngneat/spectator/jest';
import { MockDirective } from 'ng-mocks';
import { IconSize } from '../icon/icon-size';
import { ButtonRole, ButtonSize, ButtonStyle } from './button';
import { ButtonComponent } from './button.component';
Expand All @@ -12,6 +14,7 @@ describe('Button Component', () => {
const createHost = createHostFactory({
component: ButtonComponent,
imports: [ButtonModule, RouterTestingModule],
declarations: [MockDirective(TrackDirective)],
providers: [],
declareComponent: false
});
Expand Down
2 changes: 1 addition & 1 deletion projects/components/src/button/button.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ButtonRole, ButtonSize, ButtonStyle } from './button';
changeDetection: ChangeDetectionStrategy.OnPush,
template: `
<ht-event-blocker event="click" class="button-container" [enabled]="this.disabled">
<button class="button" [ngClass]="this.getStyleClasses()">
<button class="button" [ngClass]="this.getStyleClasses()" [htTrack] [htTrackLabel]="this.label">
Copy link
Contributor

Choose a reason for hiding this comment

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

do we capture something like the route path or url here? Seems like it's worth not just tracking "Submit" for example, but where that submit is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. URL change is tracked via router events and it can be enabled via enablePageTracking option

Copy link
Contributor

Choose a reason for hiding this comment

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

but those are separate events. If I'm looking at a click event for a button labeled "submit" how do I know what page it's on? Or do the tools provide that by tracking the event timeline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the things we can capture and add to the "eventData" object from inside the telemetryImpl service. But AFAIU, the telemetry tools will timestamp these anyway, so we can connect the dots there. Let's iterate over this.

<ht-icon
*ngIf="this.icon && !this.trailingIcon"
[icon]="this.icon"
Expand Down
3 changes: 2 additions & 1 deletion projects/components/src/button/button.module.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';
import { UserTelemetryTrackingModule } from '@hypertrace/common';
import { EventBlockerModule } from '../event-blocker/event-blocker.module';
import { IconModule } from '../icon/icon.module';
import { LabelModule } from '../label/label.module';
import { ButtonComponent } from './button.component';

@NgModule({
imports: [CommonModule, IconModule, LabelModule, EventBlockerModule],
imports: [CommonModule, IconModule, LabelModule, EventBlockerModule, UserTelemetryTrackingModule],
declarations: [ButtonComponent],
exports: [ButtonComponent]
})
Expand Down
7 changes: 4 additions & 3 deletions projects/components/src/link/link.component.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { RouterLinkWithHref } from '@angular/router';
import { NavigationService } from '@hypertrace/common';
import { RouterTestingModule } from '@angular/router/testing';
import { NavigationService, TrackDirective } from '@hypertrace/common';
import { createHostFactory, mockProvider, SpectatorHost } from '@ngneat/spectator/jest';
import { MockDirective } from 'ng-mocks';
import { of } from 'rxjs';
Expand All @@ -11,9 +12,9 @@ describe('Link component', () => {

const createHost = createHostFactory({
component: LinkComponent,
imports: [LetAsyncModule],
imports: [LetAsyncModule, RouterTestingModule],
providers: [mockProvider(NavigationService)],
declarations: [MockDirective(RouterLinkWithHref)]
declarations: [MockDirective(RouterLinkWithHref), MockDirective(TrackDirective)]
});

test('Link contents should be displayed if params/url is undefined', () => {
Expand Down
2 changes: 2 additions & 0 deletions projects/components/src/link/link.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { EMPTY, Observable } from 'rxjs';
[queryParamsHandling]="navData?.extras?.queryParamsHandling"
[skipLocationChange]="navData?.extras?.skipLocationChange"
[replaceUrl]="navData?.extras?.replaceUrl"
[htTrack]
htTrackLabel="{{ navData && navData.path ? navData.path : '' }}"
>
<ng-content></ng-content>
</a>
Expand Down
3 changes: 2 additions & 1 deletion projects/components/src/link/link.module.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';
import { RouterModule } from '@angular/router';
import { UserTelemetryTrackingModule } from '@hypertrace/common';
import { LetAsyncModule } from '../let-async/let-async.module';
import { LinkComponent } from './link.component';

@NgModule({
declarations: [LinkComponent],
exports: [LinkComponent],
imports: [CommonModule, RouterModule, LetAsyncModule]
imports: [CommonModule, RouterModule, LetAsyncModule, UserTelemetryTrackingModule]
})
export class LinkModule {}