Skip to content

Conversation

toddhgardner
Copy link
Member

This is a basic/manual implementation of TrackJS telemetry.

TrackJS.addTelemetry("console", {
  timestamp: timestamp(),
  severity: "log",
  message: "message"
});

Can accept console, network, navigation, or visitor events. There is no normalization or serialization on what you send (yet), since it's directly on our API, it trusts that you are not doing something silly. But maybe it should do some limiting/validation on the fields?

I also tested that an object can be updated after being added to the log. This is useful for network where it will be completed at a later time with more information.

It does not clear the Telemetry log after sending an error. I went to implement this, but noticed that the Node agent doesn't do this intentionally because we thought at the time that including all the telemetry history with the 2,3,...nth error is useful. I like this approach I think, but I wanted to call it out and run it by yall.

@BrandesEric
Copy link
Member

Looks good to me one! One thought:

Say I do something like client.addTelemetry("console", {}) - it would be cool to enforce that if you choose "console" as the first arg, the second arg MUST be of type ConsoleTelemetry. Supposedly, according to ChatGPT 5 - this might be possible to model in TS?

https://chatgpt.com/share/6899f0b4-3a08-800a-a8ef-7422887e40e6

Would be sorta cool if that's not a hallucination.

@J-Griffin
Copy link

Agree it would be nice to have the types even if there is not code verifying what is passed at runtime.

I think Eric/ChatGPT is right that you can do overloads. Something like this?

type TelemetryKind = "console" | "network" | "etc...";

interface ConsoleData {
    message: string
}

interface NetworkData {
    statusCode: number
}

function addTelemetry(kind: "console", data: ConsoleData) : void;
function addTelemetry(kind: "network", data: NetworkData) : void;
function addTelemetry(kind: TelemetryKind, data: ConsoleData | NetworkData) : void {
   // do things...
}

// Usage
addTelemetry("console", { message: "test" }); // works
addTelemetry("network", { message: "test" }); // compile error
addTelemetry("network", { statusCode: 500 }); // works

@toddhgardner
Copy link
Member Author

toddhgardner commented Aug 12, 2025

Another way we could do this is with actual types, which could be enforced at runtime.

TrackJS.addTelemetry(new ConsoleTelemetry(timestamp(), "log", "message"));

Then, rather than using the type string at all, we just use telemetry instanceOf ConsoleTelemetry and store it appropriately.

EDIT: Another benefit of this is that it gives us a good place to do validation, like saying URLs can't be 1000char+, or log messages have to be less than X"

@J-Griffin
Copy link

That might be the most straightforward way to do it.

@BrandesEric
Copy link
Member

Yep, I considered suggesting that too but figured you were keeping the type of telemetry separate from the payload maybe for serialization purposes (like that way the type name is not sent as part of the payload, just to keep it smaller and such)

I don't have a strong preference one way or another - I don't mind the way you've got it now, so with some type constraint/maps might be just fine? Otherwise if you're feeling like the "more" OOP approach approach is better that's fine with me too!

@toddhgardner
Copy link
Member Author

I went with the @BrandesEric / @J-Griffin Typescript approach. I didn't like the idea of revealing internal types like ConsoleTelemetry and making a user instantiate our types to use it.

But since the syntax really only prevented Typescript from doing something wrong, I wrote another e2e test that was just plain-old JavaScript and tried to do things incorrectly to make sure something threw an error rather than silently sending crap to our API. It's not bullet proof, but I didn't really want to build a full type-check-enforcement thingy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants