Closed Bug 1566764 Opened Last month Closed Last month

Consider allowing more characters (e.g. `-`) in the labels for labelled metrics

Categories

(Data Platform and Tools :: Glean: SDK, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Dexter, Assigned: Dexter)

References

Details

(Whiteboard: [telemetry:mobilesdk:m8][telemetry:glean-rs:m6])

Attachments

(5 files)

Thanks to Chris' investigation (bug 1552507), we noticed that Fenix is currently not reporting full search data. See the specific investigation here by boek.

This is due to the fact that Fenix is using the engine name as the label for search_count.

However search engine name, in non en-US locales, might contain characters other than the ones we allow for labels. A list of default search engines is available here. Even if we were to use the engine code instead of the name, as we do on desktop, we'd still fail to validate due to the extensive use of - in the engine code name.

This is our current regex for validating labels.

Priority: -- → P1
Whiteboard: [telemetry:mobilesdk:m8][telemetry:glean-rs:m6]

Thoughts on the above?

I think that, at the very least, we should find a way to allow '-' in label names, to keep feature parity with desktop. This might be important for analysis across desktop and mobile. The Fenix might need to switch to use 'code' instead of the engine 'name', to have a consistent behaviour with respect to Desktop.

Flags: needinfo?(tlong)
Flags: needinfo?(mdroettboom)
Flags: needinfo?(jrediger)

(uh, I typed this comment before, but it wasn't saved?)

- sounds ok to me, should be easy to handle in code. What's the pipeline's opinion on that?
What was the initial decision for such a restricted set of allowed characters (the fact that it gets transformed into a table column name?)?

Flags: needinfo?(jrediger)

(In reply to Jan-Erik Rediger [:janerik] from comment #2)

(uh, I typed this comment before, but it wasn't saved?)

- sounds ok to me, should be easy to handle in code. What's the pipeline's opinion on that?
What was the initial decision for such a restricted set of allowed characters (the fact that it gets transformed into a table column name?)?

Frank, would having a dash - in Glean SDK labels be a problem for the pipeline?

Flags: needinfo?(fbertsch)

Barring any complaints from the pipeline side of things with the -, I don't have any problems with adding dashes to the allowed separators.

Flags: needinfo?(tlong)

Labels do not transformed to columns, they are simply strings - this should be good to go for the pipeline.

Flags: needinfo?(fbertsch)
Assignee: nobody → alessio.placitelli
Flags: needinfo?(mdroettboom)

I will change glean-core as part of this bug as well.

The ones for glean-core are already bumped as part of the related PR.

Status: NEW → RESOLVED
Closed: Last month
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.