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

chore: disable telemetry #39137

Merged
merged 4 commits into from
Oct 23, 2024
Merged

chore: disable telemetry #39137

merged 4 commits into from
Oct 23, 2024

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Oct 15, 2024

Description

Telemetry endpoint was shut down and telemetry is no longer functional so this looks to produce gatsby-telemetry package with no-op functions, so that already release gatsby (and gatsby packages that use telemetry) could possibly get telemetry package upgrade and avoid POSTing to no longer working telemetry endpoint as that's just wasteful. Note that gatsby-telemetry dependency is not pinned so it's technically possible to get that new version but also likely won't happen that often as the version would be pinned by lock file. Regardless of that it seems worth to make it possible to get updated telemetry package that skips trying to hit collection endpoint.

This PR also removes "vendored" telemetry usage from create-gatsby and usage of gatsby-telemetry from other packages.

Removal of usage from packages was intentionally done in commit after making gatsby-telemetry no-op to show that both pass tests

Documentation

Tests

Related Issues

https://linear.app/netlify/issue/FRB-1396/deprecate-telemetry-functionality-in-gatsbyjs

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 15, 2024
@pieh pieh force-pushed the chore/remove-telemetry branch 2 times, most recently from 5411e21 to 3d42489 Compare October 15, 2024 16:26
@serhalp serhalp marked this pull request as ready for review October 16, 2024 17:25
serhalp
serhalp previously approved these changes Oct 16, 2024
Copy link
Contributor

@serhalp serhalp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pieh pieh force-pushed the chore/remove-telemetry branch 2 times, most recently from 24ee1ab to e24ad92 Compare October 17, 2024 07:48
@pieh pieh changed the title chore: remove telemetry chore: disable telemetry Oct 17, 2024
@pieh pieh force-pushed the chore/remove-telemetry branch from e24ad92 to b855a38 Compare October 17, 2024 08:26
@pieh pieh removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 17, 2024
Copy link
Contributor

@serhalp serhalp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines -110 to +67
return this.config.all
return this.config.all()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was "wrong" before and after some changes typescript started to complain - https://github.com/gatsbyjs/gatsby/pull/39137/files/a3b9f5628f95c35e1b8f54cde9258708fc125ac7#diff-cca3d37894655cf103c925eb2e8d20ccdab86c9fc6e1093f60777acd06be4550L20 the config could be one of two types before, but in here I did remove ConfigStore (this would be stuff that write/read some settings to ~/.config/<some-tool>) and kept just MemoryConfig which interface wasn't actually compatible apparently

}

flush()
// no-op
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file can be deleted, I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very possible, yes.

The tricky bit here is that gatsby-telemetry doesn't have well defined public API surface and some other packages use non-main modules (and there is no export map) and I just am not sure that this module was never used by other packages and decided to keep module there. This module was not used by latest versions of packages (that part I did check) but I don't know about all the previous versions within current major.

The main goal of those changes was ensuring this new version will just not POST to now disabled collection endpoint - there is still quite a bit of code that is left in this package so I think this is reasonable to keep that module in

@pieh pieh merged commit 1b1720b into master Oct 23, 2024
38 checks passed
@pieh pieh deleted the chore/remove-telemetry branch October 23, 2024 07:16
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 this pull request may close these issues.

2 participants