Closed Bug 1566359 Opened 5 months ago Closed 5 months ago

Pack the `metrics.yaml` file in the GeckoView AAR published on Maven

Categories

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

task

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Dexter, Assigned: nalexander)

References

(Blocks 1 open bug)

Details

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

Attachments

(1 file)

engine-gecko-nightly will need to consume the metrics.yaml file in mozilla-central, at build time, for the Glean SDK to work.

For this reason we need to export such a file in the AAR GeckoView package, built from mozilla-central and published over Maven, that engine-gecko depends on.

This bug is for changing the Gecko build system in order to package such file. Questions:

  • should the build fail if no metrics.yaml is found? I have the feeling that we probably shouldn't, if possible.
  • if the build will fail, then we'd need to provide an "empty" metrics.yaml at the desired location before bug 1566354 is finished.
Blocks: 1566340
Priority: -- → P3
Whiteboard: [telemetry:glean-rs:m?]

Hi Johan!

In order to support exfiltrating Gecko metrics, through GeckoView and Glean, in Fenix, we need to add a new file in the GeckoView AAR. Do you know if new AAR files are ignored by default?

See this meeting notes for more details.

Flags: needinfo?(jlorenzo)
See Also: → 1566363

Hi Alessio!

That is a very good question. I confess I don't know the answer. I looked up the Android docs and it seems metrics.yaml can fit in /assets/ because it's present and expected in the AAR file[1] + this file is meant to be compiled in the APK as is[2]. That looks like what you guys want, if I'm not mistaken. I'll defer the final call to the folks who built the Android Components, they may have better insights than I do. Sebastian, Grisha, Christian, what do you guys think?

[1] https://developer.android.com/studio/projects/android-library#aar-contents
[2] https://developer.android.com/studio/projects#ProjectView

Flags: needinfo?(s.kaspari)
Flags: needinfo?(jlorenzo)
Flags: needinfo?(gkruglov)
Flags: needinfo?(csadilek)

(In reply to Johan Lorenzo [:jlorenzo] from comment #2)

... this file is meant to be compiled in the APK as is[2]. That looks like what you guys want, if I'm not mistaken.

Thanks for following up so quickly! To be fair, we only need this file at build time, we don't need it to be in the final APK.

Correct. The file should not end up in the APK.

The only documentation about the layout I could find is this very small section:
https://developer.android.com/studio/projects/android-library#aar-contents

From that it seems putting it into the root (like proguard.txt) is probably a reasonable approach.

Flags: needinfo?(s.kaspari)
Flags: needinfo?(gkruglov)
Flags: needinfo?(csadilek)

(In reply to Alessio Placitelli [:Dexter] from comment #1)

Hi Johan!

In order to support exfiltrating Gecko metrics, through GeckoView and Glean, in Fenix, we need to add a new file in the GeckoView AAR. Do you know if new AAR files are ignored by default?

See this meeting notes for more details.

Johan: specifically, do we examine and allow/disallow certain pieces of GeckoView AAR files? (I hope not, 'cuz there are 20+ optional bits in there.)

This will add a metrics.yaml in the AAR root, which would need to be allow-listed if we have such a list.

Flags: needinfo?(jlorenzo)

Ah, I see! On releng's end, we don't check the content of the AAR file itself (unlike APKs, where we do enforce some bits to be present). So you guys can modify the content of the AAR file, the pipeline won't be busted 🙂

Flags: needinfo?(jlorenzo)

(In reply to Johan Lorenzo [:jlorenzo] from comment #6)

Ah, I see! On releng's end, we don't check the content of the AAR file itself (unlike APKs, where we do enforce some bits to be present). So you guys can modify the content of the AAR file, the pipeline won't be busted 🙂

Thanks Johan!

Nick, you mentioned knowing what to change in order to make this happen. Any chance you could guide/mentor me or kindly submit a patch for this?

The file is already on autoland and will be available in toolkit/components/telemetry/geckoview/streaming/metrics.yaml.

Flags: needinfo?(nalexander)

(In reply to Alessio Placitelli [:Dexter] from comment #7)

(In reply to Johan Lorenzo [:jlorenzo] from comment #6)

Ah, I see! On releng's end, we don't check the content of the AAR file itself (unlike APKs, where we do enforce some bits to be present). So you guys can modify the content of the AAR file, the pipeline won't be busted 🙂

Thanks Johan!

Nick, you mentioned knowing what to change in order to make this happen. Any chance you could guide/mentor me or kindly submit a patch for this?

Done.

For local testing:

nalexander@roboto ~/M/gecko> ./mach gradle geckoview:publishWithGeckoBinariesDebugPublicationToMavenRepository
...
nalexander@roboto ~/M/gecko> find ../objdirs/objdir-droid/gradle/build/mobile/android -iname '*geckoview*aar'
../objdirs/objdir-droid/gradle/build/mobile/android/geckoview/maven/org/mozilla/geckoview/geckoview-default/70.0.20190719111558/geckoview-default-70.0.20190719111558.aar
../objdirs/objdir-droid/gradle/build/mobile/android/geckoview/outputs/aar/geckoview-withGeckoBinaries-debug.aar
nalexander@roboto ~/M/gecko> unzip -l ../objdirs/objdir-droid/gradle/build/mobile/android/geckoview/outputs/aar/geckoview-withGeckoBinaries-debug.aar | grep metrics.yaml
      449  02-01-1980 00:00   metrics.yaml
nalexander@roboto ~/M/gecko> unzip -l ../objdirs/objdir-droid/gradle/build/mobile/android/geckoview/maven/org/mozilla/geckoview/geckoview-default/*/geckoview-default-*.aar | grep metrics.yaml
      449  02-01-1980 00:00   metrics.yaml

I elected to not including the variantConfiguration to support inter-project() dependency consumption. We can cross that bridge sometime as we build out the Glean SDK ecosystem.

Flags: needinfo?(nalexander)
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Priority: P3 → P1

Mmm, I see a named it Part 1. I'm happy to produce a Part 2 (with the project() bits as well), or I'll update the message. Alessio, let me know.

(In reply to Nick Alexander :nalexander [he/him] from comment #10)

Mmm, I see a named it Part 1. I'm happy to produce a Part 2 (with the project() bits as well), or I'll update the message. Alessio, let me know.

I'm fine with this being without the project() support for the moment. I'd only file a follow-up bug (here) for adding that part, too. I'm not familiar with the details for this, so I'd really appreciate if you could file it!

Thanks for the great work and efforts!

Whiteboard: [telemetry:glean-rs:m?] → [telemetry:glean-rs:m7]
Attachment #9079427 - Attachment description: Bug 1566359 - Part 1: Pack GeckoView-specific `metrics.yaml` file into published AAR. r?esawin → Bug 1566359 - Pack GeckoView-specific `metrics.yaml` file into published AAR. r?esawin
Pushed by nalexander@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3278956a87e9
Pack GeckoView-specific `metrics.yaml` file into published AAR. r=esawin
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.