Add metrics info for Mobile CSTG #585

caroline-ttd · 2024-05-26T21:49:49Z

uid2_token_response_status_count_total{advertising_token_version="V2", application="uid2-operator", cstg="true", instance="host.docker.internal:9080", job="services", platformType="HasOriginHeader", site_id="123", site_name="MegaTest Site", token_endpoint="ClientSideTokenGenerateV2", token_response_status="Success"}

uid2_token_response_status_count_total{advertising_token_version="null", application="uid2-operator", cstg="true", instance="host.docker.internal:9080", job="services", platformType="InApp", site_id="123", site_name="MegaTest Site", token_endpoint="ClientSideTokenGenerateV2", token_response_status="InvalidAppName"}

Slack discussions are here: https://thetradedesk.slack.com/archives/C051ELQ8JAC/p1716760305149009

Add the platformType for /token/client-generate, /token/refresh and /token/generate

src/main/java/com/uid2/operator/monitoring/TokenResponseStatsCollector.java

src/main/java/com/uid2/operator/vertx/UIDOperatorVerticle.java

src/main/java/com/uid2/operator/monitoring/TokenResponseStatsCollector.java

src/main/java/com/uid2/operator/vertx/UIDOperatorVerticle.java

jon8787 · 2024-05-29T07:18:46Z

src/main/java/com/uid2/operator/vertx/UIDOperatorVerticle.java

        if (siteStore == null) return "unknown"; //this is expected if CSTG is not enabled, eg for private operators

        final Site site = siteStore.getSite(siteId);
        return (site == null) ? "unknown" : site.getName();
    }

+    private TokenResponseStatsCollector.PlatformType getPlatformType(RoutingContext rc) {
+        final String clientVersion = rc.request().getHeader("X-UID2-Client-Version");
+        if (clientVersion != null && (clientVersion.contains("Android") || clientVersion.contains("ios") || clientVersion.contains("tvos"))) {


curious, what's tvos, if it's a CTV string, are there others?

tvOS is essentially iOS of Apple TV.
https://en.wikipedia.org/wiki/TvOS
depends on what os the sdk is built with the client version would say ios or tvos.

no there is no other platform string variants that will appear on X-UID2-Client-Version at this stage.

Just a suggestion not advocating hard for it: maybe we should hardcode the android/ios/tvos list somewhere and then use it like:

public static boolean containsKnownPlatformType(String str, List<String> substrings) { return substrings.stream().anyMatch(substring -> str.contains(substring)); }

?

We should definitely do that once there's a 2nd use-case. Not sure I'd use stream() though.

is "tvos" and "ios" case sensitive?

well we are using String.contains which is case sensitive ?

https://github.com/IABTechLab/uid2-ios-sdk/blob/91c290d29a7093cfc209eca493d1fee80c17e16a/Sources/UID2/UID2Client.swift#L36-L38

I added the URL as a comment and created a HashSet for "Android", "ios", "tvOS".

Does it need to be a HashSet? are we ever calling hashSet.contains() ?

Nope, we are only using String.contains SUPPORTED_IN_APP.stream().anyMatch(clientVersionHeader::contains)).

It can also be an array list, I just feel as the values are unique, maybe good to use HashSet?

I don't think we need hashSet.contains() function, I will change it to an array.

Yes, we're mainly doing iteration, with is faster with an array (not that it matters much). Uniqueness is a good feature but since we're providing all the values, we can already ensure they're unique.

jon8787 · 2024-05-30T03:57:30Z

src/main/java/com/uid2/operator/monitoring/TokenResponseStatsCollector.java

-    public static void record(ISiteStore siteStore, Integer siteId, Endpoint endpoint, TokenVersion advertisingTokenVersion, ResponseStatus responseStatus) {
-        recordInternal(siteStore, siteId, endpoint, responseStatus, advertisingTokenVersion, endpoint == Endpoint.ClientSideTokenGenerateV2);
+    public enum PlatformType {
+        InApp, // Request containing the "X-UID2-Client-Version" header, typically originating from Android, iOS, or tvOS (Apple TV).


Not just "X-UID2-Client-Version", since server and JS SDKs contain this as well. It has to specifically contain ios/android/tvos in this header.

Yeah, that makes sense.

jon8787 · 2024-05-30T03:57:48Z

src/main/java/com/uid2/operator/monitoring/TokenResponseStatsCollector.java

-        recordInternal(siteStore, siteId, endpoint, responseStatus, advertisingTokenVersion, endpoint == Endpoint.ClientSideTokenGenerateV2);
+    public enum PlatformType {
+        InApp, // Request containing the "X-UID2-Client-Version" header, typically originating from Android, iOS, or tvOS (Apple TV).
+        HasOriginHeader, // Request containing the "original" header, originating from the web.


"origin" not original :)

Thank you for catching it!

jon8787 · 2024-05-30T03:59:20Z

src/main/java/com/uid2/operator/vertx/UIDOperatorVerticle.java

@@ -1793,6 +1806,17 @@ public static String getSiteName(ISiteStore siteStore, Integer siteId) {
        return (site == null) ? "unknown" : site.getName();
    }

+    private TokenResponseStatsCollector.PlatformType getPlatformType(RoutingContext rc) {
+        final String clientVersionHeader = rc.request().getHeader("X-UID2-Client-Version");


magic string, use ClientVersionHeader

Oh, sorry, I got it. Will change.

jon8787 · 2024-05-31T02:18:39Z

src/main/java/com/uid2/operator/vertx/UIDOperatorVerticle.java

@@ -518,7 +525,7 @@ private OriginOrAppNameValidationResult validateOriginOrAppName(RoutingContext r
                    : OriginOrAppNameValidationResult.invalidAppName(appName);
        }

-        final String origin = rc.request().getHeader("origin");
+        final String origin = rc.request().getHeader(ORIGIN_HEADER);


It's actually the whole expression rc.request().getHeader(ORIGIN_HEADER); that's duplicated many times, not just the "origin" string. So we should instead have a method getOriginHeader(rc).

But that raises a question - why do we sometimes have rc.request().headers().contains(ORIGIN_HEADER)); instead of getHeader?

From the doc, getHeader vs headers looks like no difference.

rc.request().headers().contains(ORIGIN_HEADER)); from the MR.

Hi Vishal, looks like you made this change, do you have any input @vishalegbert-ttd

Add metrics info for Mobile CSTGi

6ccf14d

caroline-ttd changed the title ~~Add metrics info for Mobile CSTGi~~ Add metrics info for Mobile CSTG May 26, 2024

jon8787 reviewed May 27, 2024

View reviewed changes

src/main/java/com/uid2/operator/monitoring/TokenResponseStatsCollector.java Outdated Show resolved Hide resolved

sunnywu reviewed May 27, 2024

View reviewed changes

src/main/java/com/uid2/operator/monitoring/TokenResponseStatsCollector.java Outdated Show resolved Hide resolved

src/main/java/com/uid2/operator/monitoring/TokenResponseStatsCollector.java Outdated Show resolved Hide resolved

Add platformType for refresh and generate endpoint too

1d06065

sunnywu reviewed May 29, 2024

View reviewed changes

src/main/java/com/uid2/operator/vertx/UIDOperatorVerticle.java Show resolved Hide resolved

sunnywu reviewed May 29, 2024

View reviewed changes

src/main/java/com/uid2/operator/monitoring/TokenResponseStatsCollector.java Outdated Show resolved Hide resolved

jon8787 reviewed May 29, 2024

View reviewed changes

Address the comments and add unit tests

ae288fb

caroline-ttd requested review from sunnywu and jon8787 May 30, 2024 03:37

jon8787 reviewed May 30, 2024

View reviewed changes

Address the comments

83ea8bd

caroline-ttd requested a review from jon8787 May 31, 2024 01:35

caroline-ttd added 2 commits May 30, 2024 18:51

Remove one more magic word

09def0f

Remove extra code

454e554

jon8787 approved these changes May 31, 2024

View reviewed changes

caroline-ttd merged commit d670c47 into main May 31, 2024
6 checks passed

caroline-ttd deleted the ccm-UID2-3486-add-metrics-info-for-mobile-CSTG branch May 31, 2024 18:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metrics info for Mobile CSTG #585

Add metrics info for Mobile CSTG #585

caroline-ttd commented May 26, 2024 •

edited

Loading

jon8787 May 29, 2024

sunnywu May 29, 2024

sunnywu May 29, 2024

jon8787 May 29, 2024

sunnywu May 29, 2024

caroline-ttd May 30, 2024

jon8787 May 30, 2024

caroline-ttd May 30, 2024

caroline-ttd May 30, 2024

jon8787 May 30, 2024

jon8787 May 30, 2024

caroline-ttd May 30, 2024

jon8787 May 30, 2024

caroline-ttd May 30, 2024

jon8787 May 30, 2024 •

edited

Loading

caroline-ttd May 30, 2024

jon8787 May 31, 2024

caroline-ttd May 31, 2024 •

edited

Loading

Add metrics info for Mobile CSTG #585

Add metrics info for Mobile CSTG #585

Conversation

caroline-ttd commented May 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jon8787 May 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caroline-ttd May 31, 2024 • edited Loading

Choose a reason for hiding this comment

caroline-ttd commented May 26, 2024 •

edited

Loading

jon8787 May 30, 2024 •

edited

Loading

caroline-ttd May 31, 2024 •

edited

Loading