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

Error thrown when using @elastic/ecs-pino-format v1.4.0 #3774

Closed
1 of 3 tasks
gilmoreorless opened this issue Dec 5, 2023 · 3 comments · Fixed by #3775
Closed
1 of 3 tasks

Error thrown when using @elastic/ecs-pino-format v1.4.0 #3774

gilmoreorless opened this issue Dec 5, 2023 · 3 comments · Fixed by #3775
Assignees
Labels
agent-nodejs Make available for APM Agents project planning. community impact:high Short-term priority; add to current release, or definitely next.

Comments

@gilmoreorless
Copy link

gilmoreorless commented Dec 5, 2023

Describe the bug

Background context:

  1. PR chore(deps): bump @elastic/ecs-pino-format from 1.4.0 to 1.5.0 #3708 auto-bumped the dependency of @elastic/ecs-pino-format from 1.4.0 to 1.5.0.
  2. 1.5.0 added a new named ecsFormat export as the preferred API.
  3. chore(deps): bump @elastic/ecs-pino-format from 1.4.0 to 1.5.0 #3708 got an additional commit that used the named export instead of the old default export.
  4. This was released in elastic-apm-node v4.2.0 (but wasn't mentioned in the changelog).

The problem:

  1. Unfortunately, chore(deps): bump @elastic/ecs-pino-format from 1.4.0 to 1.5.0 #3708 only bumped package-lock.json but left package.json alone, so it still publishes a dependency on "@elastic/ecs-pino-format": "^1.4.0"
  2. Some projects can update to [email protected] but still use @elastic/[email protected], especially when package lockfiles are taken into account.
  3. Using this combination means const { ecsFormat } = require('@elastic/ecs-pino-format') returns undefined, then later throws TypeError: ecsFormat is not a function

To Reproduce

Presumably, running npm install @elastic/[email protected] in this repo then running the tests should show the error.
I can't tell for certain because trying to run this repo's tests on my M2 mac failed to pull some Docker images.

Expected behavior

Changing the package.json minimum dependency to ^1.5.0 should fix the error.

How are you starting the agent? (please tick one of the boxes)

  • Calling agent.start() directly (e.g. require('elastic-apm-node').start(...))
  • Requiring elastic-apm-node/start from within the source code
  • Starting node with -r elastic-apm-node/start

Additional context

I would have quickly raised a PR for this, but I couldn't run the tests. (Also, a single-character PR isn't worth the hoops I'd have to jump through to sign the corporate CLA, unfortunately.)

@github-actions github-actions bot added agent-nodejs Make available for APM Agents project planning. community triage labels Dec 5, 2023
@trentm trentm self-assigned this Dec 5, 2023
@trentm trentm added impact:high Short-term priority; add to current release, or definitely next. and removed triage labels Dec 5, 2023
@trentm
Copy link
Member

trentm commented Dec 5, 2023

@gilmoreorless Thanks for the excellent bug report. This is my bad. We'll get a release out soon (hopefully today) to fix this.

@trentm
Copy link
Member

trentm commented Dec 5, 2023

For others, or to possibly help searching symptoms, the failure looks something like this:

/Users/trentm/el/apm-agent-nodejs4/lib/logging.js:171
      ...ecsFormat({ apmIntegration: false }),
         ^

TypeError: ecsFormat is not a function
    at Object.createLogger (/Users/trentm/el/apm-agent-nodejs4/lib/logging.js:171:10)
    at Object.configLogger (/Users/trentm/el/apm-agent-nodejs4/lib/config/config.js:102:18)
    at new Agent (/Users/trentm/el/apm-agent-nodejs4/lib/agent.js:50:24)
    at startNTransactions (/Users/trentm/el/apm-agent-nodejs4/test/transaction-sampling.test.js:29:19)
    at Test.<anonymous> (/Users/trentm/el/apm-agent-nodejs4/test/transaction-sampling.test.js:55:17)
    at Test.run (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/test.js:113:28)
    at Immediate.next (/Users/trentm/el/apm-agent-nodejs4/node_modules/tape/lib/results.js:157:7)
    at process.processImmediate (node:internal/timers:476:21)

Node.js v18.18.2

@gilmoreorless
Copy link
Author

Thanks for the speedy resolution!

PeterEinberger pushed a commit to fpm-git/apm-agent-nodejs that referenced this issue Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning. community impact:high Short-term priority; add to current release, or definitely next.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants