Skip to content

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

Open
CptSchnitz wants to merge 1 commit into
masterfrom
logger-otel-improvements
Open

feat(js-logger): enhance async logger initialization and kept log format when otel is enabled#261
CptSchnitz wants to merge 1 commit 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');
});


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.`,
);

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