feat: allow to pass custom sdk for fullstory

[yordis]

fixes #30

Context

This change will allow passing a different client. That means you can create your own client implementation that uses Segment over Fullstory directly.

Breaking Changes

I would prefer to remove the default import since I don't want to bundle up such a dependency. Although it is less than ideal and we should avoid breaking changes, this sounds like something simple enough that upgrading is a matter of importing Fullstory SDK and passing it to the initialization.

Another proposal

Ideally, Segment would have its own integration SDK with Sentry, or even Google Analytics or any of those tools that track user events.

Packages like https://www.npmjs.com/package/analytics try to standardize across a common API since they all seem to behave really similarly.

If such new integration standardize using the following

import { Event, EventHint } from '@sentry/types';

type ExceptionProperties = {
  name?: string;
  message?: string;
};

export interface Tracker {
  track(eventName: Event, properties: ExceptionProperties): Event;
}

Then we could probably implement something that will work for most of those tools.

I am more than happy to take the lead on such a topic, just need support from Sentry on the topic, I would prefer to move the ownership under the org eventually.

We would probably want to set up a workspace so we can have multiple implementations of the tracker.

[yordis]

@leeandher could you share some love over here as well 🙏🏻 🙏🏻 🙏🏻 🙏🏻 🙏🏻

[leeandher]

@yordis Sorry for another late response, but if you could rebase this PR I can go ahead and restart the review process to get this merged!

[yordis]

Right on, let me work on this today

[yordis]

I think it is ready,

One thought that came to my mind, take it with a grain of salt since I am just want to share some thoughts out of context.

About Logging

The more logging the integrations and Sentry itself adds the bigger the bundle, and noisy the integrations become.

Ideally, pass the logger as a dependency at the very minimal (that means don't use console directly)

But in reality, most of the logging should be added by yet another closure that somebody would create because it wants logs.

For example,

const sentryProcessor = makeSentryProcessor();

const sentryProcessorWithLogger = withLogger(sentryProcessor);

// just making a point of keep wrapping thing, embracing SOLID

Default values that can't be minimize

It is always a balance between adding good defaults (in this case the baseSentryUrl) or the consumers having to know about such a thing.

The more defaults you add, the bigger the bundle especially when those default values are not applicable for whatever reason.

Same strategy before, yet another function (really minifier friendly) that give you the defaults but don't assume thing about consumers

import { makeSentryProcessor, withDefaultConfiguration } from '...'
const sentryProcessor = makeSentryProcessor(
  withDefaultConfiguration({
    //....
  })
)

People don't read docs

No question ask, don't have an answer other than training people and make documentation that makes more sense like following, https://diataxis.fr/ we wrote an ADR trying to help on the topic: https://github.com/straw-hat-team/adr/tree/master/adrs/3223845695

[yordis]

Please double-check me in the PR.

Remember we need to remove the default client, but it would require a breaking change, or we can expose two clients, where one client does have the import type of thing.

[leeandher]

@yordis Changes look good to me! Thanks for the types and util file additions as well, it's a lot more readable. For now, I think the client as a default option is a trade-off we're going to make as we want to avoid breaking changes if possible. Bandwidth on the integration team is stretched across multiple services external to Sentry, and we don't have capacity to go forward with a project like that just yet.

Thanks for sharing your thoughts though and being super patient with our review process!