Skip to content
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

Error telemetry in cloud #3516

Merged
merged 7 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
10 changes: 0 additions & 10 deletions admin/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
runtimeauth "github.com/rilldata/rill/runtime/server/auth"
"github.com/rs/cors"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
"go.opentelemetry.io/otel/attribute"
"go.uber.org/zap"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -300,15 +299,6 @@ func (s *Server) Ping(ctx context.Context, req *adminv1.PingRequest) (*adminv1.P
return resp, nil
}

func (s *Server) Telemetry(ctx context.Context, req *adminv1.TelemetryRequest) (*adminv1.TelemetryResponse, error) {
dims := make([]attribute.KeyValue, 0)
for k, v := range req.Event {
dims = append(dims, attribute.String(k, v))
}
s.uiActivity.Emit(ctx, "cloud-ui-telemetry", 1, dims...)
return &adminv1.TelemetryResponse{}, nil
}

func timeoutSelector(fullMethodName string) time.Duration {
return time.Minute
}
Expand Down
37 changes: 37 additions & 0 deletions admin/server/telemetry.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package server

import (
"context"

adminv1 "github.com/rilldata/rill/proto/gen/rill/admin/v1"
"go.opentelemetry.io/otel/attribute"
)

func (s *Server) Telemetry(ctx context.Context, req *adminv1.TelemetryRequest) (*adminv1.TelemetryResponse, error) {
dims := make([]attribute.KeyValue, 0)
for k, v := range req.Event.AsMap() {
a, ok := toKeyValue(k, v)
if ok {
dims = append(dims, a)
}
}
s.uiActivity.Emit(ctx, req.Name, float64(req.Value), dims...)
return &adminv1.TelemetryResponse{}, nil
}

func toKeyValue(k string, v interface{}) (attribute.KeyValue, bool) {
switch vt := v.(type) {
case string:
return attribute.String(k, vt), true
case bool:
return attribute.Bool(k, vt), true
case int:
return attribute.Int(k, vt), true
case int64:
return attribute.Int64(k, vt), true
case float64:
return attribute.Float64(k, vt), true
}

return attribute.KeyValue{}, false
}
10 changes: 8 additions & 2 deletions proto/gen/rill/admin/v1/admin.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2482,10 +2482,16 @@ definitions:
v1TelemetryRequest:
type: object
properties:
name:
type: string
title: Name passed to activity module's name arg
value:
type: number
format: float
title: Value passed to activity module's value arg
event:
type: object
additionalProperties:
type: string
title: Free form struct of the actual event
v1TelemetryResponse:
type: object
v1TriggerReconcileResponse:
Expand Down
1,880 changes: 948 additions & 932 deletions proto/gen/rill/admin/v1/api.pb.go

Large diffs are not rendered by default.

33 changes: 32 additions & 1 deletion proto/gen/rill/admin/v1/api.pb.validate.go

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

7 changes: 6 additions & 1 deletion proto/rill/admin/v1/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,12 @@ message GenerateReportYAMLResponse {
}

message TelemetryRequest {
map<string, string> event = 1;
// Name passed to activity module's name arg
string name = 1;
// Value passed to activity module's value arg
float value = 2;
// Free form struct of the actual event
google.protobuf.Struct event = 3;
}

message TelemetryResponse {}
Expand Down
4 changes: 3 additions & 1 deletion web-admin/src/client/gen/index.schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,11 @@ export interface V1TelemetryResponse {
[key: string]: any;
}

export type V1TelemetryRequestEvent = { [key: string]: string };
export type V1TelemetryRequestEvent = { [key: string]: any };

export interface V1TelemetryRequest {
name?: string;
value?: number;
event?: V1TelemetryRequestEvent;
}

Expand Down
60 changes: 58 additions & 2 deletions web-admin/src/features/errors/error-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { goto } from "$app/navigation";
import { page } from "$app/stores";
import { getScreenNameFromPage } from "@rilldata/web-admin/features/navigation/nav-utils";
import { errorEvent } from "@rilldata/web-common/metrics/initMetrics";
import type { Query } from "@tanstack/query-core";
import type { QueryClient } from "@tanstack/svelte-query";
import type { AxiosError } from "axios";
import { get } from "svelte/store";
Expand All @@ -9,12 +12,29 @@ import { ADMIN_URL } from "../../client/http-client";
import { ErrorStoreState, errorStore } from "./error-store";

export function createGlobalErrorCallback(queryClient: QueryClient) {
return (error: AxiosError) => {
return (error: AxiosError, query: Query) => {
const isProjectPage = get(page).route.id === "/[organization]/[project]";
const isDashboardPage =
get(page).route.id === "/[organization]/[project]/[dashboard]";

if (!error.response) return;
console.log("createGlobalErrorCallback");
AdityaHegde marked this conversation as resolved.
Show resolved Hide resolved
const screenName = getScreenNameFromPage(get(page));
if (!error.response) {
errorEvent?.fireErrorBoundaryEvent(
query.queryKey[0] as string,
"",
"unknown error",
screenName
);
return;
} else {
errorEvent?.fireErrorBoundaryEvent(
query.queryKey[0] as string,
error.response?.status + "" ?? error.status,
(error.response?.data as RpcStatus)?.message ?? error.message,
screenName
);
}

// Special handling for some errors on the Project page
if (isProjectPage && error.response?.status === 400) {
Expand Down Expand Up @@ -130,3 +150,39 @@ export function createErrorPagePropsFromRoutingError(
body: "Try refreshing the page, and reach out to us if that doesn't fix the error.",
};
}

export function addJavascriptErrorListeners() {
const errorHandler = (errorEvt: ErrorEvent) => {
console.log("error", errorEvt);
AdityaHegde marked this conversation as resolved.
Show resolved Hide resolved
errorEvent?.fireErrorBoundaryEvent(
"",
"",
errorEvt.error?.stack ?? errorEvt.message,
getScreenNameFromPage(get(page))
);
};
const unhandledRejectionHandler = (rejectionEvent: PromiseRejectionEvent) => {
console.log("rejection", rejectionEvent);
AdityaHegde marked this conversation as resolved.
Show resolved Hide resolved
let reason = "";
if (typeof rejectionEvent.reason === "string") {
reason = rejectionEvent.reason;
} else if (rejectionEvent.reason instanceof Error) {
reason = rejectionEvent.reason.stack;
} else {
reason = String.toString.apply(rejectionEvent.reason);
}
errorEvent?.fireErrorBoundaryEvent(
"",
"",
reason,
getScreenNameFromPage(get(page))
);
};

window.addEventListener("error", errorHandler);
window.addEventListener("unhandledrejection", unhandledRejectionHandler);
return () => {
window.removeEventListener("error", errorHandler);
window.removeEventListener("unhandledrejection", unhandledRejectionHandler);
};
}
24 changes: 24 additions & 0 deletions web-admin/src/features/navigation/nav-utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { MetricsEventScreenName } from "@rilldata/web-common/metrics/service/MetricsTypes";
import type { Page } from "@sveltejs/kit";

export function isOrganizationPage(page: Page): boolean {
Expand All @@ -18,3 +19,26 @@ export function isDashboardPage(page: Page): boolean {
export function isReportPage(page: Page): boolean {
return page.route.id === "/[organization]/[project]/-/reports/[report]";
}

export function isReportExportPage(page: Page): boolean {
return (
page.route.id ===
"/[organization]/[project]/[dashboard]/-/reports/[report]/export"
);
}

export function getScreenNameFromPage(page: Page): MetricsEventScreenName {
switch (true) {
case isOrganizationPage(page):
return MetricsEventScreenName.Organization;
case isProjectPage(page):
return MetricsEventScreenName.Project;
case isDashboardPage(page):
return MetricsEventScreenName.Dashboard;
case isReportPage(page):
return MetricsEventScreenName.Report;
case isReportExportPage(page):
return MetricsEventScreenName.ReportExport;
}
return MetricsEventScreenName.Home;
ericpgreen2 marked this conversation as resolved.
Show resolved Hide resolved
}
23 changes: 23 additions & 0 deletions web-admin/src/features/telemetry/RillAdminTelemetryClient.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { ADMIN_URL } from "@rilldata/web-admin/client/http-client";
import type { MetricsEvent } from "@rilldata/web-common/metrics/service/MetricsTypes";
import type { TelemetryClient } from "@rilldata/web-common/metrics/service/RillIntakeClient";

export class RillAdminTelemetryClient implements TelemetryClient {
public async fireEvent(event: MetricsEvent) {
try {
const resp = await fetch(`${ADMIN_URL}/v1/telemetry`, {
method: "POST",
body: JSON.stringify({
name: event.app_name + "-ui-telemetry",
value: 1,
event,
}),
credentials: "include",
});
if (!resp.ok)
console.error(`Failed to send ${event.event_type}. ${resp.statusText}`);
} catch (err) {
console.error(`Failed to send ${event.event_type}. ${err.message}`);
}
}
}
26 changes: 26 additions & 0 deletions web-admin/src/features/telemetry/initCloudMetrics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { RillAdminTelemetryClient } from "@rilldata/web-admin/features/telemetry/RillAdminTelemetryClient";
import { collectCommonUserFields } from "@rilldata/web-common/metrics/collectCommonUserFields";
import { ErrorEventHandler } from "@rilldata/web-common/metrics/ErrorEventHandler";
import {
metricsService,
setErrorEvent,
setMetricsService,
} from "@rilldata/web-common/metrics/initMetrics";
import { BehaviourEventFactory } from "@rilldata/web-common/metrics/service/BehaviourEventFactory";
import { ErrorEventFactory } from "@rilldata/web-common/metrics/service/ErrorEventFactory";
import { MetricsService } from "@rilldata/web-common/metrics/service/MetricsService";
import { ProductHealthEventFactory } from "@rilldata/web-common/metrics/service/ProductHealthEventFactory";

export async function initCloudMetrics() {
setMetricsService(
new MetricsService(new RillAdminTelemetryClient(), [
new ProductHealthEventFactory(),
new BehaviourEventFactory(),
new ErrorEventFactory(),
])
);

const commonUserMetrics = await collectCommonUserFields();
setErrorEvent(new ErrorEventHandler(metricsService, commonUserMetrics));
// TODO: add other handlers and callers
}
10 changes: 9 additions & 1 deletion web-admin/src/routes/+layout.svelte
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
<script lang="ts">
import { beforeNavigate } from "$app/navigation";
import { initCloudMetrics } from "@rilldata/web-admin/features/telemetry/initCloudMetrics";
import NotificationCenter from "@rilldata/web-common/components/notifications/NotificationCenter.svelte";
import {
featureFlags,
retainFeaturesFlags,
} from "@rilldata/web-common/features/feature-flags";
import RillTheme from "@rilldata/web-common/layout/RillTheme.svelte";
import { QueryClient, QueryClientProvider } from "@tanstack/svelte-query";
import { createGlobalErrorCallback } from "../features/errors/error-utils";
import { onMount } from "svelte";
import {
addJavascriptErrorListeners,
createGlobalErrorCallback,
} from "../features/errors/error-utils";
import ErrorBoundary from "../features/errors/ErrorBoundary.svelte";
import TopNavigationBar from "../features/navigation/TopNavigationBar.svelte";
import { clearViewedAsUserAfterNavigate } from "../features/view-as-user/clearViewedAsUser";
Expand Down Expand Up @@ -37,6 +42,9 @@

beforeNavigate(retainFeaturesFlags);
clearViewedAsUserAfterNavigate(queryClient);
initCloudMetrics();

onMount(() => addJavascriptErrorListeners());
ericpgreen2 marked this conversation as resolved.
Show resolved Hide resolved
</script>

<svelte:head>
Expand Down
11 changes: 11 additions & 0 deletions web-admin/src/routes/[organization]/[project]/+layout.svelte
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
<script lang="ts">
import { goto } from "$app/navigation";
import { page } from "$app/stores";
import { createAdminServiceGetCurrentUser } from "@rilldata/web-admin/client";
import ProjectDashboardsListener from "@rilldata/web-admin/features/projects/ProjectDashboardsListener.svelte";
import { metricsService } from "@rilldata/web-common/metrics/initMetrics";
import RuntimeProvider from "@rilldata/web-common/runtime-client/RuntimeProvider.svelte";
import { isProjectPage } from "../../../features/navigation/nav-utils";
import ProjectTabs from "../../../features/projects/ProjectTabs.svelte";
Expand All @@ -12,6 +14,7 @@
$page.params.organization,
$page.params.project
);
const user = createAdminServiceGetCurrentUser();

$: isRuntimeHibernating = $projRuntime.isSuccess && !$projRuntime.data;

Expand All @@ -21,6 +24,14 @@
}

$: onProjectPage = isProjectPage($page);

$: if ($page.params.project && $user.data?.user?.id) {
metricsService.loadCloudFields({
isDev: window.location.host.startsWith("localhost"),
projectId: $page.params.project,
userId: $user.data?.user?.id,
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. What about tracking the organizationId too?
  2. Can we load the userId from the AvatarButton.svelte component? Because the userID will be present if the user is logged in on the Home and Org pages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added organizationId. It might be better to load all the data at a single place.

</script>

{#if $viewAsUserStore}
Expand Down
Loading