Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

glean/experiments: Hard dependency on lib-httpurlconnection #6660

Closed
pocmo opened this issue Apr 15, 2020 · 3 comments · Fixed by #6875
Closed

glean/experiments: Hard dependency on lib-httpurlconnection #6660

pocmo opened this issue Apr 15, 2020 · 3 comments · Fixed by #6875
Labels
⌨️ code Technical debt, code clean up, small API change .. <glean> Component: service-glean (New telemetry SDK)

Comments

@pocmo
Copy link
Contributor

pocmo commented Apr 15, 2020

I was investigating why we still see (I think unused) fragments of lib-httpurlconnection in Fenix APKs. I noticed that service-glean as well as service-experiments have a hard (non-test) dependency on lib-httpurlconnection.

My assumption is that we can get rid of that dependency and instead always enforce passing in a lib-fetch implementation.

┆Issue is synchronized with this Jira Task

@pocmo pocmo added ⌨️ code Technical debt, code clean up, small API change .. <glean> Component: service-glean (New telemetry SDK) <experiments> labels Apr 15, 2020
@travis79
Copy link
Member

Sorry we missed this getting filed. I filed Bug 1634068 in Bugzilla so that this will be triaged in our next meeting.

@badboy
Copy link
Member

badboy commented May 5, 2020

Glean does actively depend on HttpURLConnectionClient to implement its uploader. Same for experiments.

Both want to provide a default, so users don't need to care about it (but can still change it).

@pocmo: What do you propose as the way forward if a hard dependency on lib-fetch-httpurlconnection is not desired?

@pocmo
Copy link
Contributor Author

pocmo commented May 5, 2020

I think we should avoid having a default inside android-components (glean of course can have a default itself). We desperately want to avoid using anything but Gecko for networking and those defaults just hide what is being used. It is so easy to not know about them.

badboy added a commit to badboy/android-components that referenced this issue May 5, 2020
We still test with it, so we keep it around.
There is no change for users:

* If they configure an HTTP client it will be used
* If they don't, the default of Glean proper will be used (which is a
  small shim around java.net.HttpURLConnection itself)

Fixes mozilla-mobile#6660 (at least partly)
badboy added a commit to badboy/android-components that referenced this issue May 6, 2020
We still test with it, so we keep it around.

*BREAKING CHANGE*:

Users will need to supply a configuration with a ping uploader to Glean
now.
A default lib-fetch-httpurlconnection implementation can be used like
this:

```kotlin
import mozilla.components.lib.fetch.httpurlconnection.HttpURLConnectionClient;
import mozilla.components.service.glean.config.Configuration;
import mozilla.components.service.glean.net.ConceptFetchHttpUploader;

val config = Configuration(httpClient = ConceptFetchHttpUploader(lazy { HttpURLConnectionClient() }))
Glean.initialize(context, true, config)
```

For Java this becomes:

```java
import mozilla.components.lib.fetch.httpurlconnection.HttpURLConnectionClient;
import mozilla.components.service.glean.config.Configuration;
import mozilla.components.service.glean.net.ConceptFetchHttpUploader;

ConceptFetchHttpUploader httpClient = ConceptFetchHttpUploader.fromClient(new HttpURLConnectionClient());
Configuration config = new Configuration(httpClient);
Glean.INSTANCE.initialize(context, true, config);
```

Fixes mozilla-mobile#6660 (at least partly)
@bors bors bot closed this as completed in 6719e01 May 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⌨️ code Technical debt, code clean up, small API change .. <glean> Component: service-glean (New telemetry SDK)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants