Pack the `metrics.yaml` file in the GeckoView AAR published on Maven
Categories
(Data Platform and Tools :: Glean: SDK, task, P1)
Tracking
(Not tracked)
People
(Reporter: Dexter, Assigned: nalexander)
References
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.
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 1•5 years ago
|
||
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.
Comment 2•5 years ago
|
||
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
Reporter | ||
Comment 3•5 years ago
|
||
(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.
Comment 4•5 years ago
|
||
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.
Updated•5 years ago
|
Assignee | ||
Comment 5•5 years ago
|
||
(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.
Comment 6•5 years ago
|
||
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 🙂
Reporter | ||
Comment 7•5 years ago
|
||
(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
.
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
(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.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 10•5 years ago
|
||
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.
Reporter | ||
Comment 11•5 years ago
|
||
(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!
Reporter | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 12•5 years ago
|
||
Comment 13•5 years ago
|
||
bugherder |
Description
•