feat(geo): Add GeoIp lookup to light normalization #2229
Conversation
This change introduces the geoip lookup to lightnormalization process, which allows up to populate the `User` with correct geo information as soon as we can.
| @@ -551,13 +551,13 @@ impl EnvelopeProcessorService { | |||
| project_cache: Addr<ProjectCache>, | |||
| upstream_relay: Addr<UpstreamRelay>, | |||
| ) -> anyhow::Result<Self> { | |||
| let geoip_lookup = match config.geoip_path() { | |||
There was a problem hiding this comment.
This relies on processing being active and requires a change in PoP configuration. Do PoP relays have the processing flag enabled in this case (and behave pretty much like processing relays)? If yes, have we identified the side effects of this config change? I'd like to avoid undesired side effects by PoPs attempting to behave like processing relays after this config change.
There was a problem hiding this comment.
you're right , the processing had to be set to enabled, otherwise it fails, but in bc416d4 I make a change to make sure we can set the processing options even if the processing.enabled isn't set, which makes it easy to configure geoip db for the pops.
|
Re-requesting review , since added few more changes to the config part |
This change introduces the GeoIp lookup to light normalization process, which allows us to populate the
Userwith correct geo information as soon as we can. Which also allow to set a correct tag in metrics extraction , if the metrics extraction is happening on our side (in the PoPs in this case).I still keep the GeoIp lookup in the store normalization, since it's also used in few places, more specifically in
relay-cabi, which exposed few different functions to the python lib.Lookup won't be triggered in any case if the Geo information is either provided by user or set on any of the steps.
related to #2225