[editor][client] Enable Telemetry based on User Config settings #899

Ankush-lastmile · 2024-01-11T23:05:41Z

[editor][client] Enable Telemetry based on User Config settings

This diff builds on top of @rholinshead's starting point for telemetry diff. Most of that looked good to me so I didn't touch it beside @rossdanlm 's .

Building off of [AIC-py][editor] server: persistent local cfg + telemetry flag, endpoints #869, if allow_usage_data_sharing is set to True, initialize Datadog Browser logging with a session ID.
Disabled telemetry in dev mode as per @rholinshead 's comment that the hot reload spams the logs with something like "datadog logging already initialized"
Moved the initialization logic away fromindex.tsx into editor.tsx so that it can be configurable.

Testplan:

yarn build
run in "prod" mode
Edit AIconfig
Hit Save button -> validate datadog log sent

repeat with aiconfig.rc set with false logging

|
| -> No logs |

I tried taking a video but datadog doesn't immediately update with logs which made the video too long to upload

Stack created with Sapling. Best reviewed with ReviewStack.

python/src/aiconfig/editor/client/src/LocalEditor.tsx

rossdanlm · 2024-01-11T23:17:52Z

This diff builds on top of @rholinshead's starting point for telemetry diff.

Can you link diff?

python/src/aiconfig/editor/client/src/LocalEditor.tsx

rossdanlm

Generally lgtm, just some comments and nits

rossdanlm · 2024-01-11T23:19:55Z

python/src/aiconfig/editor/client/src/LocalEditor.tsx

+        service: "aiconfig-editor",
+        site: "us5.datadoghq.com",
+        forwardErrorsToLogs: true,
+        sessionSampleRate: 100,


In diff summary, can you link to website for API docs so we can read more on what these mean?

whoops forgot to link docs. Generally, datadog docs are easily googleable and easily navigate able. Can just google "datadog <search term>"

"datadog browser logging" -> https://docs.datadoghq.com/logs/log_collection/javascript/

python/src/aiconfig/editor/client/src/utils/logging.ts

python/src/aiconfig/editor/client/src/shared/types.ts

python/src/aiconfig/editor/client/src/LocalEditor.tsx

python/src/aiconfig/editor/client/src/components/AIConfigContext.tsx

python/src/aiconfig/editor/client/src/components/AIConfigEditor.tsx

python/src/aiconfig/editor/client/src/LocalEditor.tsx

Ankush-lastmile · 2024-01-13T00:29:25Z

removed custom sessionID because @rholinshead pointed out that datadog already creates one by default.
only log in "prod" mode, as @rholinshead pointed out, in dev mode we would be spamming the logs when hot reloads occur
added in check for aiconfigrc
addressed some nits

Ankush-lastmile · 2024-01-13T00:34:22Z

Can you link diff?

Its the diff below this one, sapling created a new pr

rholinshead

LGTM, but let's do the dev check & return before sending the request unnecessarily

python/src/aiconfig/editor/client/src/LocalEditor.tsx

rholinshead · 2024-01-13T01:03:14Z

python/src/aiconfig/editor/client/src/LocalEditor.tsx

+    const res = await ufetch.get(ROUTE_TABLE.GET_AICONFIGRC, {}); // change this to the endpoint used to get the config
+
+    const enableTelemetry = res.allow_usage_data_sharing;
+    const isDev = (process.env.NODE_ENV ?? "development") === "development";


Let's do the dev check at the top so we don't need to make a request unnecessarily

Checked for isDev before making the request, and if so return early.

Manually tested the flows and verified telemetry is only sent in prod and not dev

rholinshead · 2024-01-13T01:04:59Z

python/src/aiconfig/editor/client/src/shared/types.ts

@@ -61,3 +61,6 @@ export function aiConfigToClientConfig(aiconfig: AIConfig): ClientAIConfig {
    },
  };
 }
+
+export type LogEvent = "ADD_PROMPT" | "SAVE_BUTTON_CLICKED";
+export type LogEventData = JSONObject;


We should, TODOs were mainly from when it was pre-release

rholinshead · 2024-01-13T01:05:17Z

python/src/aiconfig/editor/client/src/utils/api.ts

@@ -14,6 +14,7 @@ export const ROUTE_TABLE = {
  CANCEL: urlJoin(API_ENDPOINT, "/cancel"),
  CLEAR_OUTPUTS: urlJoin(API_ENDPOINT, "/clear_outputs"),
  DELETE_PROMPT: urlJoin(API_ENDPOINT, "/delete_prompt"),
+  GET_AICONFIGRC: urlJoin(API_ENDPOINT, "/get_aiconfigrc"),


What does rc mean?

fairly certain that rc stands for run commands. Most people will have multiple of these on their system already, you can see which rc files you have on your own computer by running this in terminal ls -a $HOME | grep rc.

To define if telemetry is to be disabled, user will need to define their own .aiconfigrc. Will need to add this to docs as well

rebased version of #804 onto main

@rholinshead

This diff builds on top of @rholinshead's starting point for telemetry diff. Most of that looked good to me so I didn't touch it beside @rossdanlm 's . - Building off of #869, if `allow_usage_data_sharing` is set to True, initialize Datadog Browser logging with a session ID. - Disabled telemetry in dev mode as per @rholinshead 's comment that the hot reload spams the logs with something like "datadog logging already initialized" - Moved the initialization logic away from`index.tsx` into `editor.tsx` so that it can be configurable. ## Testplan: 1. yarn build 2. run in "prod" mode 3. Edit AIconfig 4. Hit Save button -> validate datadog log sent repeat with aiconfig.rc set with false logging |<img width="963" alt="Screenshot 2024-01-12 at 7 24 33 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/c2f979df-327e-40c4-9f06-034531437a65"> | <img width="1455" alt="Screenshot 2024-01-12 at 7 23 51 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/edb022b5-4abd-4ddc-b9bb-97713d32e5dc"> | | ------------- | ------------- | | <img width="604" alt="Screenshot 2024-01-12 at 7 26 35 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/4a2e49a3-061e-404c-9681-13a15aca8e9c"> | -> No logs | I tried taking a video but datadog doesn't immediately update with logs which made the video too long to upload

Ankush-lastmile · 2024-01-14T02:22:03Z

moved isDev check to occur before making the request to get the aiconfigrc data. This way the request is only made if needed
- Manually tested the flows and verified telemetry is only sent in prod and not dev

This was referenced Jan 11, 2024

[editor][client] helper util to generate a session id #898

Closed

[editor] Starting Point for Telemetry #864

Merged

Ankush-lastmile force-pushed the pr899 branch 4 times, most recently from 21065be to 56d69aa Compare January 11, 2024 23:12

Ankush-lastmile commented Jan 11, 2024

View reviewed changes

python/src/aiconfig/editor/client/src/LocalEditor.tsx Show resolved Hide resolved

Ankush-lastmile force-pushed the pr899 branch from 56d69aa to 2ac5a00 Compare January 11, 2024 23:17

rossdanlm reviewed Jan 11, 2024

View reviewed changes

python/src/aiconfig/editor/client/src/LocalEditor.tsx Show resolved Hide resolved

rossdanlm approved these changes Jan 11, 2024

View reviewed changes

rholinshead reviewed Jan 12, 2024

View reviewed changes

python/src/aiconfig/editor/client/src/LocalEditor.tsx Outdated Show resolved Hide resolved

Ankush-lastmile mentioned this pull request Jan 12, 2024

{WIP} default enable_telemetry is False, env info #865

Closed

Ankush-lastmile force-pushed the pr899 branch from 2ac5a00 to 5d2decc Compare January 13, 2024 00:28

Ankush-lastmile marked this pull request as ready for review January 13, 2024 00:34

Ankush-lastmile requested review from saqadri, suyoglastmileai and jonathanlastmileai as code owners January 13, 2024 00:34

rholinshead approved these changes Jan 13, 2024

View reviewed changes

Ryan Holinshead added 2 commits January 13, 2024 21:04

[editor] Starting Point for Telemetry

7d7be6d

rebased version of #804 onto main

Ankush-lastmile force-pushed the pr899 branch from 5d2decc to d881775 Compare January 14, 2024 02:19

Ankush-lastmile mentioned this pull request Jan 14, 2024

Schematize editor telemetry logging payload #915

Open

Ankush-lastmile merged commit f110958 into main Jan 14, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[editor][client] Enable Telemetry based on User Config settings #899

[editor][client] Enable Telemetry based on User Config settings #899

Ankush-lastmile commented Jan 11, 2024 •

edited

Loading

rossdanlm commented Jan 11, 2024

rossdanlm left a comment

rossdanlm Jan 11, 2024

Ankush-lastmile Jan 13, 2024 •

edited

Loading

Ankush-lastmile commented Jan 13, 2024 •

edited

Loading

Ankush-lastmile commented Jan 13, 2024

rholinshead left a comment

rholinshead Jan 13, 2024

Ankush-lastmile Jan 14, 2024

rholinshead Jan 13, 2024

rholinshead Jan 13, 2024

Ankush-lastmile Jan 14, 2024

Ankush-lastmile commented Jan 14, 2024

[editor][client] Enable Telemetry based on User Config settings #899

[editor][client] Enable Telemetry based on User Config settings #899

Conversation

Ankush-lastmile commented Jan 11, 2024 • edited Loading

Testplan:

rossdanlm commented Jan 11, 2024

rossdanlm left a comment

Choose a reason for hiding this comment

rossdanlm Jan 11, 2024

Choose a reason for hiding this comment

Ankush-lastmile Jan 13, 2024 • edited Loading

Choose a reason for hiding this comment

Ankush-lastmile commented Jan 13, 2024 • edited Loading

Ankush-lastmile commented Jan 13, 2024

rholinshead left a comment

Choose a reason for hiding this comment

rholinshead Jan 13, 2024

Choose a reason for hiding this comment

Ankush-lastmile Jan 14, 2024

Choose a reason for hiding this comment

rholinshead Jan 13, 2024

Choose a reason for hiding this comment

rholinshead Jan 13, 2024

Choose a reason for hiding this comment

Ankush-lastmile Jan 14, 2024

Choose a reason for hiding this comment

Ankush-lastmile commented Jan 14, 2024

Ankush-lastmile commented Jan 11, 2024 •

edited

Loading

Ankush-lastmile Jan 13, 2024 •

edited

Loading

Ankush-lastmile commented Jan 13, 2024 •

edited

Loading