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

feat: Migrate to Node context #866

Merged
merged 8 commits into from
Apr 3, 2024
Merged

feat: Migrate to Node context #866

merged 8 commits into from
Apr 3, 2024

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Apr 3, 2024

Closes #525
Closes #603

This PR migrates to using device context mostly supplied by the nodeContextIntegration rather than the similar implementation that it was originally forked from in the Electron SDK.

This PR:

  • Moves language to culture context (Add support for Culture Context #525)
  • Removes user.ip_address: '{{auto}}' which should not be included by default (user.ip_address should not be set to {{auto}} #603)
  • Removes the event_type tag which was replaced with event.environment in v4
  • The mainContextIntegration has been removed and replaced by:
    • electronContextIntegration which both adds Electron context and removes Node context which is not appropriate
    • normalizePathsIntegration which normalises all paths
  • All context is now applied to events through event processors

@timfish timfish marked this pull request as ready for review April 3, 2024 12:34
@timfish timfish enabled auto-merge (squash) April 3, 2024 13:27
@timfish
Copy link
Collaborator Author

timfish commented Apr 3, 2024

For some reason the tests for v15.5.7 on ubuntu pass but the process hangs at the end and doesn't exit 😭

@AbhiPrasad
Copy link
Member

Stale promise somewhere?

might be useful to throw a https://www.npmjs.com/package/why-is-node-running at it and see what gets logged out.

Side note - can we max times for the tests? Ideally we don't use 5 hours for the runner, we can just cancel the tests after 30 min.

@timfish
Copy link
Collaborator Author

timfish commented Apr 3, 2024

can we max times for the tests?

Yeah it looks like you can set a timeout for actions steps and 25m should be plenty.

@timfish timfish merged commit 360288c into getsentry:master Apr 3, 2024
25 of 32 checks passed
@timfish
Copy link
Collaborator Author

timfish commented Apr 3, 2024

I have no idea how I managed to merge this. I didn't click anything and all the tests weren't passing. I enabled auto merge earlier today 🤷‍♂️

@timfish timfish deleted the context branch April 3, 2024 23:30
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.

user.ip_address should not be set to {{auto}} Add support for Culture Context
2 participants