1704193 - No way to verify label correctness for LabeledMetricType

Status: RESOLVED DUPLICATE of bug 1672273

(Data Platform and Tools :: Glean: SDK, enhancement, P3)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I'm using a LabeledMetricType<CounterMetricType>. Since the labels are passed in as strings, e.g.:

PerfStartup.startupType["cold_main"]

I want to ensure the strings I pass in are valid labels. The best way I can think to do this is write a getLabel() function for my metric and, in a test, ensure all possible outputs are valid, e.g.:

// production code
fun getLabel(arg) { ... }

// test code
@Test
fun ensureAllLabelsAreValid() {
    val allOutputs = allValidValuesForArgs.map { getLabel(it) }
    allOutputs.forEach { 
        assertTrue(PerfStartup.startupType.testIsLabelValid(it))
    }
}

However, afaict, it is not possible with the Glean SDK to determine if a label is valid despite valid labels being collected in metrics.yaml. I would like to be able to determine if a label is valid so I can ensure, in testing, that I will not transmit bad data.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I thought I might be able to do something with testGetNumRecordedErrors (which isn't the most intuitive solution but could work) but it didn't seem to work:

@Test
fun testGetNumRecordedErrors() {
        PerfStartup.startupType["haha"].add()
        assertEquals(0, PerfStartup.startupType.testGetNumRecordedErrors(ErrorType.InvalidLabel))
        assertEquals(0, PerfStartup.startupType.testGetNumRecordedErrors(ErrorType.InvalidOverflow))
        assertEquals(0, PerfStartup.startupType.testGetNumRecordedErrors(ErrorType.InvalidState))
        assertEquals(0, PerfStartup.startupType.testGetNumRecordedErrors(ErrorType.InvalidValue))
        assertEquals(0, PerfStartup.startupType["haha"].testGetNumRecordedErrors(ErrorType.InvalidLabel))
        assertEquals(0, PerfStartup.startupType["haha"].testGetNumRecordedErrors(ErrorType.InvalidOverflow))
        assertEquals(0, PerfStartup.startupType["haha"].testGetNumRecordedErrors(ErrorType.InvalidState))
        assertEquals(0, PerfStartup.startupType["haha"].testGetNumRecordedErrors(ErrorType.InvalidValue))
}

I'd expect this test to fail because "haha" isn't a valid label but it passes. Is there a bug in testGetNumRecordedErrors? Or perhaps I'm not using the "pingName" argument correctly.

Comment 2

[Chris H-C :chutten]

I can give you an answer, but you probably won't like it : )

"invalid" labels get mapped to __other__. So what happens is PerfStartup.startupType["haha"] -> PerfStartup.startupType["__other__"] and then it all works.

You can validate this by adding to a second invalid label and seeing that "haha"'s count increases.

(( This doesn't help with the ergonomics problem you highlight, but might help with understanding the rules that this is following at the moment ))

Comment 3

[Chris H-C :chutten]

bug 1672273 may have helped out here by reducing the label format to be "71 or fewer characters of printable ASCII".

There are some aspects of this bug that aren't quite handled (like maybe you don't want to check each and every label for length or happen to know the right way to check if the characters are printable ASCII), but enough of it ought to be that I feel okay closing this down.