Skip to content

fix: Use custom poller connectionId #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

anna-gn
Copy link

@anna-gn anna-gn commented Apr 2, 2025

About the changes

When a custom poller is provided, use its connectionId to set the connectionId in UnleashClientBase, instead of generating one.
UnleashClientBase.connectionId is used when reporting metrics. In existing implementation, when a custom poller is provided to UnleashClientBase.init, the poller and the client IDs mismatch.

Instead of generating a new connectionId, use the connectionId of a custom poller, whenever it's provided, to set correct values in metrics
@gastonfournier
Copy link

Hi @anna-gn my initial response would be that we don't want this to happen. What's the need for this request?

Connection Id is something internal for Unleash and shouldn't be overridden. You can find this documented here: https://docs.getunleash.io/reference/sdks#sdk-identification-headers

@anna-gn
Copy link
Author

anna-gn commented Apr 3, 2025

Hi @gastonfournier, I assume that without this fix our metrics won't be correctly attributed to our client apps and users.
The documentation says:

The same connection ID should be used for each request when an SDK instance polls the API

More on our use case:
to be able to use a custom storage implementation, we create our own Poller instance, for which we generate a connectionId, since it's required by the initializer:


The poller is then provided to the UnleashClient instance in its initializer:

This results in different connection IDs for the Poller and the Client.

Connection Id is something internal for Unleash and shouldn't be overridden.

As an alternative, would it be better to provide an overriding API for the poller, other than in the client initializer? In our case, it will also work if we could just override the poller storage.

@gastonfournier
Copy link

metrics won't be correctly attributed to our client apps and users

Client app should be identified by their appName, not the clientConnectionId. In a single App you can have multiple instances running of the app, each should have a unique connectionId. We can't guarantee that if we allow users to override it.

As an alternative, would it be better to provide an overriding API for the poller, other than in the client initializer? In our case, it will also work if we could just override the poller storage.

Yes, this could be on possible solution IMO. An alternative would potentially be that we expose the assigned connection id so you can use it, but maybe your proposal seems a bit simpler, reducing the API surface that you have to implement.

I'll bring this to the team

@gastonfournier gastonfournier moved this from New to In Progress in Issues and PRs Apr 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants