Skip to content
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

Allow cdn in AnalyticsSettings #362

Closed
chambo-e opened this issue Jan 12, 2022 · 16 comments
Closed

Allow cdn in AnalyticsSettings #362

chambo-e opened this issue Jan 12, 2022 · 16 comments

Comments

@chambo-e
Copy link

chambo-e commented Jan 12, 2022

Hello 👋

If I understand correctly the current way to set the cdn is this

window.analytics._cdn = <cdn>

Because this variable can then be reused in the plugins

We try to avoid touching the window object as much as possible
Would it be possible to pass the cdn in the AnalyticsSettings which is then passed down to the plugins ?

AnalyticsBrowser.load({ writeKey, cdn })
export function loadLegacySettings(writeKey: string, cdn: string): Promise<LegacySettings> {
  return fetch(`${cdn}/v1/projects/${writeKey}/settings`)
    .then((res) => ({ ...res.json(), cdn }))
    .catch((err) => {
      console.warn('Failed to load settings', err)
      throw err
    })
}

Thanks :)

@pooyaj
Copy link
Contributor

pooyaj commented Jan 12, 2022

Hi @chambo-e, good suggestion. We are planning to improve the configurability of the library for npm users. Took a note of the _cdn variable as well to be included in the scope. This won't land immediately, but we are pushing to ship this quarter.

@chambo-e
Copy link
Author

chambo-e commented Feb 2, 2022

Thanks :)

By any chance do you have a public roadmap which would allow us to close this issue ?

@yordis
Copy link
Contributor

yordis commented Mar 23, 2022

Any updates on this one?

@pooyaj
Copy link
Contributor

pooyaj commented Mar 23, 2022

Thanks for checking! We just released this feature in 1.34.0.
More info:
PR: #389
Release: https://github.com/segmentio/analytics-next/releases/tag/v1.34.0

@pooyaj pooyaj closed this as completed Mar 23, 2022
@yordis
Copy link
Contributor

yordis commented Mar 23, 2022

@pooyaj I am a bit confused because I do want to hit the CDN to retrieve such information just that we need to use our Proxy Server to bypass problems with Ad Blockers and whatnot.

Are you suggesting that we do the CDN call on our side now?

@pooyaj
Copy link
Contributor

pooyaj commented Mar 23, 2022

@yordis my bad closing this, as passing hardcoded settings won't solve this particular issue. I think we still need to implement passing custom CDN endpoint for proxy users 😅 hopefully coming up soon!

@pooyaj pooyaj reopened this Mar 23, 2022
yordis added a commit to yordis/analytics-next that referenced this issue Mar 24, 2022
yordis added a commit to yordis/analytics-next that referenced this issue Mar 24, 2022
yordis added a commit to yordis/analytics-next that referenced this issue Mar 24, 2022
@yordis
Copy link
Contributor

yordis commented Mar 24, 2022

@pooyaj I created a PR for it, let me know if that works

@vicpara
Copy link

vicpara commented Apr 7, 2022

I am using the v1.35.0 of @analytics-next that includes this fix. I can see on the network how settings is retrieved from the custom domain cdn via the cdnURL property.

On the other hand, we are using Segment with other 3rd party providers (eg. MouseFlow ). Even though the CDN is correctly configured I can see on the network how other 3rd party resources are still retrieved from segment CDN instead of using the customer proxy url. (eg. https://cdn.segment.com/next-integrations/integrations/mouseflow/2.2.3/mouseflow.dynamic.js.gz )

Do you have any further guidance on how to force the integrations to rely on the custom proxy cdn ?

Thanks

@silesky
Copy link
Contributor

silesky commented Apr 7, 2022

I am using the v1.35.0 of @analytics-next that includes this fix. I can see on the network how settings is retrieved from the custom domain cdn via the cdnURL property.

On the other hand, we are using Segment with other 3rd party providers (eg. MouseFlow ). Even though the CDN is correctly configured I can see on the network how other 3rd party resources are still retrieved from segment CDN instead of using the customer proxy url. (eg. https://cdn.segment.com/next-integrations/integrations/mouseflow/2.2.3/mouseflow.dynamic.js.gz )

Do you have any further guidance on how to force the integrations to rely on the custom proxy cdn ?

Thanks

@vicpara thanks for bringing this up, we're looking into this. In the meantime, does this work (assuming React)?

const useAnalytics = (writeKey: string) => {
  const [analytics, setAnalytics] = React.useState<Analytics | undefined>(
    undefined
  );
  useEffect(() => {
    const loadAnalytics = async () => {
      try {
        if (staging) {
          (globalThis as any).analytics = {
            _cdn: "https://cdn.foo.com",
          };
        }
       const [response, ctx] = await AnalyticsBrowser.load({
          writeKey,
        });
        setAnalytics(response);
      } catch (err) {
        console.error(err);
      }
    };
    loadAnalytics();
  }, [writeKey]);
    return analytics;
};

@vicpara
Copy link

vicpara commented Apr 7, 2022

@silesky I am loading analytics-next in React via npm package manager and I set the cdn in the .load method like shown below. This works well for Segment artifacts but not for the 3rd party plugins. In this I followed your support team's guidance.

    AnalyticsBrowser.load(
        {
          writeKey: '12..34',
          cdnURL: 'https://cdn.customdomain.com',
        },
        {
          integrations: {
            'Segment.io': {
              apiHost: 'api.customdomain.com/v1',
            },
          },
        }
      )

I checked 'https://github.com/segmentio/analytics.js-integrations/blob/master/integrations/mouseflow/lib/index.js' which shows that the actual integration has a hardcoded url.

  1. I'm not sure if this link from analytics.js-integration is still relevant to this particular topic.
  2. Wanted to check you don't have any magic flag or setting that would rewrite the plugin source based on the CDN hostname.

@silesky
Copy link
Contributor

silesky commented Apr 7, 2022

Wanted to check you don't have any magic flag or setting that would rewrite the plugin source based on the CDN hostname.

@vicpara Thanks for sleuthing; maybe @pooyaj can chime in on and confirm if this is something that's possible anywhere in Segment?

@pooyaj
Copy link
Contributor

pooyaj commented Apr 7, 2022

hey @vicpara, the expected behavior for setting cdnURL is that all assets that were previously being loaded from cdn.segment.com should now be loaded from cdn.customdomain.com including mouseflow.dynamic.js.gz. But the actual mouseflow SDK ( that is referenced in the plugin source code ) will still load from cdn.mouseflow.com.

@pooyaj
Copy link
Contributor

pooyaj commented Apr 7, 2022

If you see mouseflow.dynamic.js.gz is being loaded from cdn.segment.com then we have to take a look at it

@vicpara
Copy link

vicpara commented Apr 7, 2022

@pooyaj Thanks for your answer and for looking into this.

Currently I see the following assets still being downloaded from the segment cdn:
https://cdn.segment.com/next-integrations/integrations/mouseflow/2.2.3/mouseflow.dynamic.js.gz
https://cdn.segment.com/next-integrations/integrations/vendor/commons.5xxxxxxe.js.gz

commons.5xxxxxxe is an actual longer string that looks like it can be used as an identifier. I redacted it for this reason. If it helps I am happy to share it with you.

@pooyaj
Copy link
Contributor

pooyaj commented Apr 7, 2022

I see! then it is a bug on our end 😅 we will look into it and hopefully should be able to release the fix

@silesky
Copy link
Contributor

silesky commented Apr 8, 2022

@vicpara working on this in #424

adisbanda pushed a commit to juneHQ/june-analytics-next that referenced this issue Jul 13, 2022
…tio#414)

* feat: add support for configuring CDN

fixes segmentio#362

* feat: continue adding support for configuring CDN, fixes segmentio#362

Co-authored-by: Yordis Prieto Lazo <yordis.prieto@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants