application-services icon indicating copy to clipboard operation
application-services copied to clipboard

Adding Glean back to the iOS megazord

Open travis79 opened this issue 3 years ago • 4 comments

This is so we can use it to record metrics in Rust on iOS

travis79 avatar Jun 22 '22 18:06 travis79

Codecov Report

Base: 41.45% // Head: 41.28% // Decreases project coverage by -0.17% :warning:

Coverage data is based on head (c7b69c6) compared to base (3e570ba). Patch coverage: 72.72% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5009      +/-   ##
==========================================
- Coverage   41.45%   41.28%   -0.18%     
==========================================
  Files         167      168       +1     
  Lines       12620    12628       +8     
==========================================
- Hits         5232     5213      -19     
- Misses       7388     7415      +27     
Impacted Files Coverage Δ
components/nimbus/build.rs 100.00% <ø> (ø)
components/nimbus/src/dbcache.rs 67.30% <0.00%> (-5.61%) :arrow_down:
components/nimbus/src/lib.rs 64.07% <40.00%> (-0.30%) :arrow_down:
components/nimbus/src/enrollment.rs 94.90% <100.00%> (+0.02%) :arrow_up:
components/nimbus/tests/test_telemetry.rs 100.00% <100.00%> (ø)
...ponents/external/glean/glean-core/build/src/lib.rs 0.00% <0.00%> (-92.00%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar Jun 22 '22 19:06 codecov-commenter

There's one more thing to do for this, and that's add test coverage for the metric collection in Rust, I ran into some issues with that and need to speak with Jan-Erik to see what's going on.

It also seems to fail some checks due to the import! macro in glean_metrics.rs 🤔

travis79 avatar Jun 22 '22 19:06 travis79

Seems I forgot to move the exposure event to Rust completely for iOS... not quite ready yet.

travis79 avatar Jun 23 '22 22:06 travis79

Adding the do-not-merge label until we get ready to land this for v107 nightlies (mid-September)

travis79 avatar Aug 18 '22 20:08 travis79

@travis79 since this has gotten a little stale, do you know what the priority of this work is now? (i.e do we expect we'll get to it anytime soon?)

I don't object to keeping the PR open until we get to it, just curious what a timeline might look like if we have one?

tarikeshaq avatar Feb 22 '23 19:02 tarikeshaq

@tarikeshaq Yes, the priorities on this shifted a bit. This is still desirable to do but I don't have time to get back to it immediately. Ultimately I'd love to be able to solve both iOS and Android when we get back to this. Let's close this in favor of starting fresh without any bitrot to deal with.

travis79 avatar Feb 23 '23 14:02 travis79