Telemetry implementation - #4

Open
toddhgardner wants to merge 5 commits into
mainfrom
telemetry
Open

Telemetry implementation#4
toddhgardner wants to merge 5 commits into
mainfrom
telemetry

Conversation

@toddhgardner

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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?

typeTelemetryKind="console"|"network"|"etc...";interfaceConsoleData{message: string}interfaceNetworkData{statusCode: number}functionaddTelemetry(kind: "console",data: ConsoleData) : void;functionaddTelemetry(kind: "network",data: NetworkData) : void;functionaddTelemetry(kind: TelemetryKind,data: ConsoleData|NetworkData) : void{// do things...}// UsageaddTelemetry("console",{message: "test"});// worksaddTelemetry("network",{message: "test"});// compile erroraddTelemetry("network",{statusCode: 500});// works

@toddhgardner

toddhgardner commented Aug 12, 2025

Copy link
Copy Markdown
MemberAuthor

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

TrackJS.addTelemetry(newConsoleTelemetry(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
Copy Markdown

That might be the most straightforward way to do it.

@BrandesEric

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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 freeto 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

@toddhgardner@BrandesEric@J-Griffin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Telemetry implementation - #4

Open
toddhgardner wants to merge 5 commits into
mainfrom
telemetry
Open

Telemetry implementation#4
toddhgardner wants to merge 5 commits into
mainfrom
telemetry

Conversation

@toddhgardner

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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?

typeTelemetryKind="console"|"network"|"etc...";interfaceConsoleData{message: string}interfaceNetworkData{statusCode: number}functionaddTelemetry(kind: "console",data: ConsoleData) : void;functionaddTelemetry(kind: "network",data: NetworkData) : void;functionaddTelemetry(kind: TelemetryKind,data: ConsoleData|NetworkData) : void{// do things...}// UsageaddTelemetry("console",{message: "test"});// worksaddTelemetry("network",{message: "test"});// compile erroraddTelemetry("network",{statusCode: 500});// works

@toddhgardner

toddhgardner commented Aug 12, 2025

Copy link
Copy Markdown
MemberAuthor

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

TrackJS.addTelemetry(newConsoleTelemetry(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
Copy Markdown

That might be the most straightforward way to do it.

@BrandesEric

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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 freeto 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

@toddhgardner@BrandesEric@J-Griffin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Telemetry implementation - #4

Open
toddhgardner wants to merge 5 commits into
mainfrom
telemetry
Open

Telemetry implementation#4
toddhgardner wants to merge 5 commits into
mainfrom
telemetry

Conversation

@toddhgardner

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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?

typeTelemetryKind="console"|"network"|"etc...";interfaceConsoleData{message: string}interfaceNetworkData{statusCode: number}functionaddTelemetry(kind: "console",data: ConsoleData) : void;functionaddTelemetry(kind: "network",data: NetworkData) : void;functionaddTelemetry(kind: TelemetryKind,data: ConsoleData|NetworkData) : void{// do things...}// UsageaddTelemetry("console",{message: "test"});// worksaddTelemetry("network",{message: "test"});// compile erroraddTelemetry("network",{statusCode: 500});// works

@toddhgardner

toddhgardner commented Aug 12, 2025

Copy link
Copy Markdown
MemberAuthor

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

TrackJS.addTelemetry(newConsoleTelemetry(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
Copy Markdown

That might be the most straightforward way to do it.

@BrandesEric

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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 freeto 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

@toddhgardner@BrandesEric@J-Griffin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Telemetry implementation - #4

Open
toddhgardner wants to merge 5 commits into
mainfrom
telemetry
Open

Telemetry implementation#4
toddhgardner wants to merge 5 commits into
mainfrom
telemetry

Conversation

@toddhgardner

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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?

typeTelemetryKind="console"|"network"|"etc...";interfaceConsoleData{message: string}interfaceNetworkData{statusCode: number}functionaddTelemetry(kind: "console",data: ConsoleData) : void;functionaddTelemetry(kind: "network",data: NetworkData) : void;functionaddTelemetry(kind: TelemetryKind,data: ConsoleData|NetworkData) : void{// do things...}// UsageaddTelemetry("console",{message: "test"});// worksaddTelemetry("network",{message: "test"});// compile erroraddTelemetry("network",{statusCode: 500});// works

@toddhgardner

toddhgardner commented Aug 12, 2025

Copy link
Copy Markdown
MemberAuthor

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

TrackJS.addTelemetry(newConsoleTelemetry(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
Copy Markdown

That might be the most straightforward way to do it.

@BrandesEric

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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 freeto 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

@toddhgardner@BrandesEric@J-Griffin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Telemetry implementation - #4

Open
toddhgardner wants to merge 5 commits into
mainfrom
telemetry
Open

Telemetry implementation#4
toddhgardner wants to merge 5 commits into
mainfrom
telemetry

Conversation

@toddhgardner

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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?

typeTelemetryKind="console"|"network"|"etc...";interfaceConsoleData{message: string}interfaceNetworkData{statusCode: number}functionaddTelemetry(kind: "console",data: ConsoleData) : void;functionaddTelemetry(kind: "network",data: NetworkData) : void;functionaddTelemetry(kind: TelemetryKind,data: ConsoleData|NetworkData) : void{// do things...}// UsageaddTelemetry("console",{message: "test"});// worksaddTelemetry("network",{message: "test"});// compile erroraddTelemetry("network",{statusCode: 500});// works

@toddhgardner

toddhgardner commented Aug 12, 2025

Copy link
Copy Markdown
MemberAuthor

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

TrackJS.addTelemetry(newConsoleTelemetry(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
Copy Markdown

That might be the most straightforward way to do it.

@BrandesEric

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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 freeto 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

@toddhgardner@BrandesEric@J-Griffin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Telemetry implementation - #4

Open
toddhgardner wants to merge 5 commits into
mainfrom
telemetry
Open

Telemetry implementation#4
toddhgardner wants to merge 5 commits into
mainfrom
telemetry

Conversation

@toddhgardner

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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?

typeTelemetryKind="console"|"network"|"etc...";interfaceConsoleData{message: string}interfaceNetworkData{statusCode: number}functionaddTelemetry(kind: "console",data: ConsoleData) : void;functionaddTelemetry(kind: "network",data: NetworkData) : void;functionaddTelemetry(kind: TelemetryKind,data: ConsoleData|NetworkData) : void{// do things...}// UsageaddTelemetry("console",{message: "test"});// worksaddTelemetry("network",{message: "test"});// compile erroraddTelemetry("network",{statusCode: 500});// works

@toddhgardner

toddhgardner commented Aug 12, 2025

Copy link
Copy Markdown
MemberAuthor

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

TrackJS.addTelemetry(newConsoleTelemetry(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
Copy Markdown

That might be the most straightforward way to do it.

@BrandesEric

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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 freeto 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

@toddhgardner@BrandesEric@J-Griffin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Telemetry implementation - #4

Open
toddhgardner wants to merge 5 commits into
mainfrom
telemetry
Open

Telemetry implementation#4
toddhgardner wants to merge 5 commits into
mainfrom
telemetry

Conversation

@toddhgardner

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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?

typeTelemetryKind="console"|"network"|"etc...";interfaceConsoleData{message: string}interfaceNetworkData{statusCode: number}functionaddTelemetry(kind: "console",data: ConsoleData) : void;functionaddTelemetry(kind: "network",data: NetworkData) : void;functionaddTelemetry(kind: TelemetryKind,data: ConsoleData|NetworkData) : void{// do things...}// UsageaddTelemetry("console",{message: "test"});// worksaddTelemetry("network",{message: "test"});// compile erroraddTelemetry("network",{statusCode: 500});// works

@toddhgardner

toddhgardner commented Aug 12, 2025

Copy link
Copy Markdown
MemberAuthor

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

TrackJS.addTelemetry(newConsoleTelemetry(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
Copy Markdown

That might be the most straightforward way to do it.

@BrandesEric

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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 freeto 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

@toddhgardner@BrandesEric@J-Griffin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Telemetry implementation - #4

Open
toddhgardner wants to merge 5 commits into
mainfrom
telemetry
Open

Telemetry implementation#4
toddhgardner wants to merge 5 commits into
mainfrom
telemetry

Conversation

@toddhgardner

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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?

typeTelemetryKind="console"|"network"|"etc...";interfaceConsoleData{message: string}interfaceNetworkData{statusCode: number}functionaddTelemetry(kind: "console",data: ConsoleData) : void;functionaddTelemetry(kind: "network",data: NetworkData) : void;functionaddTelemetry(kind: TelemetryKind,data: ConsoleData|NetworkData) : void{// do things...}// UsageaddTelemetry("console",{message: "test"});// worksaddTelemetry("network",{message: "test"});// compile erroraddTelemetry("network",{statusCode: 500});// works

@toddhgardner

toddhgardner commented Aug 12, 2025

Copy link
Copy Markdown
MemberAuthor

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

TrackJS.addTelemetry(newConsoleTelemetry(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
Copy Markdown

That might be the most straightforward way to do it.

@BrandesEric

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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 freeto 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

@toddhgardner@BrandesEric@J-Griffin