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(v8/deps): Upgrade all OpenTelemetry dependencies #15098

Merged
merged 5 commits into from
Jan 31, 2025

Conversation

nwalters512
Copy link
Contributor

@nwalters512 nwalters512 commented Jan 20, 2025

Backport of #14967 to v8. The diff here will shrink a lot when #15094 is merged.

@nwalters512
Copy link
Contributor Author

The tests here are expected to fail until #15094 lands.

@nwalters512
Copy link
Contributor Author

@AbhiPrasad hello again! Wondering if you can offer any insight into why the suites/tracing/tedious/test.ts integration test is failing. It fails for me locally too, but only when I run the suite as a whole. If I run yarn test suites/tracing/tedious/test.ts to run it in isolation, it fails.

I added some logging of expected and envelope here:

if (expectedType !== envelopeItemType) {
throw new Error(`Expected envelope item type '${expectedType}' but got '${envelopeItemType}'`);
}

It looks like the connection to the mssql server is failing:

              breadcrumbs: [
                {
                  timestamp: 1737481330.83,
                  category: 'console',
                  level: 'error',
                  message: "ConnectionError: Login failed for user 'sa'.\n" +
                    '    at Login7TokenHandler.onErrorMessage (/Users/nathan/git/sentry-javascript/node_modules/tedious/lib/token/handler.js:186:19)\n' +
                    '    at Readable.<anonymous> (/Users/nathan/git/sentry-javascript/node_modules/tedious/lib/token/token-stream-parser.js:19:33)\n' +
                    '    at Readable.emit (node:events:519:28)\n' +
                    '    at addChunk (node:internal/streams/readable:559:12)\n' +
                    '    at readableAddChunkPushObjectMode (node:internal/streams/readable:536:3)\n' +
                    '    at Readable.push (node:internal/streams/readable:391:5)\n' +
                    '    at nextAsync (node:internal/streams/from:194:22)\n' +
                    '    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {\n' +
                    "  code: 'ELOGIN'\n" +
                    '}'
                }
              ],

@AbhiPrasad
Copy link
Member

closing up some of my other tasks for the day - this is top of my todo alongside getting #15119 to a mergeable state tomorrow morning.

Thanks again for all the PRs and effort @nwalters512! If you want some Sentry swag, please shoot me an email at a + prasad + @ + sentry.io (remove the pluses) and we'll get something to you 😄

@AbhiPrasad AbhiPrasad self-assigned this Jan 21, 2025
@klippx
Copy link

klippx commented Jan 31, 2025

Thanks @nwalters512 you are a champion! 🏆

@andreiborza andreiborza marked this pull request as ready for review January 31, 2025 15:35
Copy link
Member

@andreiborza andreiborza left a comment

Choose a reason for hiding this comment

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

Tests are now passing, this backport LGTM.

Thanks so much for this @nwalters512!

@andreiborza andreiborza merged commit 3673689 into getsentry:v8 Jan 31, 2025
147 checks passed
@nwalters512 nwalters512 deleted the v8-upgrade-otel-deps branch January 31, 2025 15:56
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.

4 participants