Skip to content

feat(js-logger): enhance async logger initialization and kept log format when otel is enabled#261

Merged
CptSchnitz merged 2 commits into
masterfrom
logger-otel-improvements
Jul 23, 2026
Merged

feat(js-logger): enhance async logger initialization and kept log format when otel is enabled#261
CptSchnitz merged 2 commits into
masterfrom
logger-otel-improvements

Conversation

@CptSchnitz

Copy link
Copy Markdown
Collaborator

No description provided.

@CptSchnitz
CptSchnitz requested a review from ronenkapelian July 15, 2026 10:46
@CptSchnitz
CptSchnitz force-pushed the logger-otel-improvements branch from 3fed889 to 62e52f2 Compare July 15, 2026 10:59

@ronenkapelian ronenkapelian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. no test for new feature:
it('should still write to the local destination when opentelemetry is enabled', async function () {
  const logger = await jsLogger({ opentelemetryOptions: { enabled: true } }, 'avi-otel.log');
  logger.info('avi otel');
  await waitForFileCreation('avi-otel.log');
  const logLine = JSON.parse(readFileSync('avi-otel.log', { encoding: 'utf-8' })) as Record<string, unknown>;
  expect(logLine).toHaveProperty('msg', 'avi otel');
});

Comment thread packages/js-logger/src/index.ts Outdated

finalLogger.debug({
msg: 'logger initialized',
level: pinoOptions.level ?? 'info',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

collides with pino's reserved level key
it seems it will print twice "level" key
here is generated script that demonstrate it

/**
 * Demonstrates finding #2 from pr-261-js-logger-async-init-otel.md:
 * the "logger initialized" debug line in src/index.ts sets a `level`
 * property on the merging object, which collides with pino's own
 * reserved `level` output key.
 *
 * Run from packages/js-logger:
 *   node scripts/demo-level-key-collision.js
 */
const pino = require('pino');

console.log('--- raw pino: what happens when you merge an object with its own "level" key ---\n');

const logger = pino({ level: 'debug' });

// This mirrors exactly what src/index.ts does:
//   finalLogger.debug({ msg: 'logger initialized', level: pinoOptions.level ?? 'info', ... })
logger.debug({ msg: 'logger initialized', level: 'info', otelEnabled: true });

console.log('\n--- look at the raw line above: count the "level" keys ---');
console.log('there are two: pino\'s real numeric level (20 = debug), and the fake string "info"\n');

console.log('--- now show what a JSON consumer actually sees after parsing ---\n');

// Capture what pino would actually write, then parse it like a log shipper would.
const chunks = [];
const captured = pino({ level: 'debug' }, { write: (chunk) => chunks.push(chunk) });
captured.debug({ msg: 'logger initialized', level: 'info', otelEnabled: true });

const rawLine = chunks.join('');
const parsed = JSON.parse(rawLine);

console.log('raw line written by pino:');
console.log(rawLine);
console.log('\nafter JSON.parse (what Loki/ELK/Datadog/etc. would ingest):');
console.log(parsed);
console.log(
  `\n=> parsed.level is ${JSON.stringify(parsed.level)} (a string), the real numeric severity (20) is gone.`,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

please mind @ronenkapelian comment

@CptSchnitz
CptSchnitz merged commit dbcc44a into master Jul 23, 2026
6 checks passed
@CptSchnitz
CptSchnitz deleted the logger-otel-improvements branch July 23, 2026 10:17
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.

3 participants