Closed Bug 1588060 Opened 2 years ago Closed 2 years ago

Provide better better metric names when using Glean metrics in Java

Categories

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

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Dexter, Assigned: brizental)

Details

(Whiteboard: [telemetry:glean-rs:backlog])

Attachments

(1 file)

Consider the following metric sample:

metric.category:
sample_string:
type: string
...

When using this metric in Java, it needs to be accessed this way:

MetricCategory.INSTANCE.getSampleString().set("test-value");

Note how the JVM produced a default accessor name for the metric, getSampleString. In Kotlin and Swift, this is just sampleString. We might be able to make this a bit better by annotating the generated code with @get:JvmName as follows:

@get:JvmName("sampleString")
val sampleString: StringMetricType by lazy {
StringMetricType(
..
)
}

This would make it callable as:

MetricCategory.INSTANCE.sampleString().set("test-value");

Basically, it will drop the "get" prefix.

Priority: -- → P3
Whiteboard: [telemetry:glean-rs:m?]
Whiteboard: [telemetry:glean-rs:m?] → [telemetry:glean-rs:m11]
Whiteboard: [telemetry:glean-rs:m11] → [telemetry:glean-rs:backlog]
Priority: P3 → P1

Alright, I have started looking at this and it is a really simple change in the parser. I am wondering what will be the impact of this changes on other projects since it is a breaking change in the Kotlin generated code.

I have taken a look and in a-c the metrics seem to never be accessed in Java, and in the Glean repo the only occurance of it is in the documentation.

My question is: should I go ahead with it even though it is a breaking change? And also, what is the procedure since this is a breaking change?

Flags: needinfo?(alessio.placitelli)

(In reply to Beatriz Rizental from comment #1)

My question is: should I go ahead with it even though it is a breaking change? And also, what are is the procedure since this is a breaking change?

How is this a breaking change? Shouldn't this be about adding a few annotations on the types?

Flags: needinfo?(alessio.placitelli) → needinfo?(brizental)
Flags: needinfo?(brizental)

I meant a breaking change in the Java API, sorry for being unclear :)

Attached file GitHub Pull Request
Assignee: nobody → brizental
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.