LG-5256: Add logging for help center troubleshooting links

[aduth]

Why: To better understand if users are interacting with these links, we want to log interactions with links shown in troubleshooting options.

[zachmargolis]

What if we added a ?source=idp to the URLs and relied on the USA Analytics, so we don't obscure the URL?

[aduth]

What if we added a ?source=idp to the URLs and relied on the USA Analytics, so we don't obscure the URL?

Do you mean as an alternative implementation? I see the upside in retaining the original URL, but a concern could be that it fragments where the event details are found and makes it difficult to include in other queries.

Another idea could be to have front-end logging when the link is clicked, though I dunno how well that would work with e.g. right-click + open in new tab, etc.

[zachmargolis]

What if we added a ?source=idp to the URLs and relied on the USA Analytics, so we don't obscure the URL?

Do you mean as an alternative implementation? I see the upside in retaining the original URL, but a concern could be that it fragments where the event details are found and makes it difficult to include in other queries.

Yup! That was my thinking, avoid a redirect by adding info in the URL. Agree that it could fragment analytics since we typically don't search there.

Another idea could be to have front-end logging when the link is clicked, though I dunno how well that would work with e.g. right-click + open in new tab, etc.

My guess is that the redirect-style logging would be more reliable than front-end logging, so I would choose that instead if we don't want to add query params like this

[aduth]

Yup! That was my thinking, avoid a redirect by adding info in the URL. Agree that it could fragment analytics since we typically don't search there.

I think we'll want to have these in CloudWatch, especially in comparing them relative to existing "Return to SP" events which are also shown in these troubleshooting options lists of links, and potentially to factor into funnel queries, or to troubleshoot an individual user's experience. But I'll raise it with the team tomorrow to see what they think.

[aduth]

Another option which at least helps keep the URL more clearly visible could be to include the full URL of the redirect, URL-encoded as a parameter (with validation, ofc)?

e.g. https://secure.login.gov/redirect?url=https%3A%2F%2Flogin.gov%2Fhelp%2Fverify-your-identity%2Faccepted-state-issued-identification%2F

Not sure it's much better, though.

[zachmargolis]

Another option which at least helps keep the URL more clearly visible could be to include the full URL of the redirect, URL-encoded as a parameter (with validation, ofc)?

e.g. https://secure.login.gov/redirect?url=https%3A%2F%2Flogin.gov%2Fhelp%2Fverify-your-identity%2Faccepted-state-issued-identification%2F

Not sure it's much better, though.

Yeah I don't think that's necessarily better, even though you mentioned it would be validated, it looks like an open redirect vulnerability and we probably don't want that attention

[aduth]

it looks like an open redirect vulnerability and we probably don't want that attention

Hah, I hadn't considered that, but very good point to avoid some future headaches 😅

[zachmargolis]

LGTM

[zachmargolis]

to me this seems more like a build method than a get method?

[aduth]

to me this seems more like a build method than a get method?

Hm, could you explain more? Is the idea that a get-prefixed method would only return a value directly, not derive something? I guess I've not made much a distinction on this previously, or at least would be happy to just offer up the value directly if that's what a "get" would imply.

I don't feel too strongly, but more worry that we haven't made much of a distinction in the past, so an argument could be made to the point of "consistency for consistency's sake" 🤷

[zachmargolis]

in my head, "get" is like "retrieve" like a load from a database or a hash or something

whereas in this case, we're not retrieving anything, we're building a new string based on some immediate values

But this is just personal style, like you said, we don't have a codebase style so I really don't feel that strongly about enforcing it

[aduth]

Okay, I think I'll plan to keep it as-is for now, though perhaps separately we can think about / document naming conventions around methods.