Skip to content

chore(bqjdbc): switch connection identifier to UUID#12957

Open
keshavdandeva wants to merge 3 commits intomainfrom
jdbc/logging-uuid
Open

chore(bqjdbc): switch connection identifier to UUID#12957
keshavdandeva wants to merge 3 commits intomainfrom
jdbc/logging-uuid

Conversation

@keshavdandeva
Copy link
Copy Markdown
Contributor

@keshavdandeva keshavdandeva commented Apr 29, 2026

b/507856382

Changes

BigQueryConnection

  • Generates a full UUID.randomUUID().toString() as the connectionId.

BigQueryJdbcMdc

  • Prepends BQ-JDBC- to the full UUID for thread context logging.

PerConnectionFileHandler

  • Extracts UUID from BQ-JDBC- prefix and constructs log filenames with the format: BQ-JDBC-yyyyMMddHHmmss-<first-4-chars-of-UUID>.log.

Tests

  • Updated BigQueryJdbcMdcTest to match the new prefix.
  • Refactored PerConnectionFileHandlerTest to use a helper method for finding dynamic log files, making tests cleaner.

@keshavdandeva keshavdandeva requested review from a team as code owners April 29, 2026 19:06
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the BigQueryJdbcMdc class to use UUIDs instead of a sequential AtomicLong for generating connection IDs. This change ensures more robust unique identification for JDBC connections when a specific ID is not provided. I have no feedback to provide.

@keshavdandeva keshavdandeva requested a review from Neenu1995 April 29, 2026 19:09
@keshavdandeva keshavdandeva requested a review from logachev April 30, 2026 12:20
@keshavdandeva keshavdandeva requested a review from Neenu1995 April 30, 2026 16:39
(id != null && !id.isEmpty()) ? id : String.valueOf(nextId.getAndIncrement());
return "JdbcConnection-" + suffix;
String baseId = (id != null && !id.isEmpty()) ? id : UUID.randomUUID().toString();
return "BQ-JDBC-" + baseId;
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.

nit: don't think we need BQ-JDBC- or any other prefix.. They're internal and only relevant to stitch together telemetry/logs. Can be just a uuid

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.

The user application many have logs generated from other sources. For the file name I think it is useful if we keep the BQ-JDBC prefix. Same with the connection ID, if the user Application is logging from multiple sources , it will be helpful to identify the logs generated by the JDBC Driver.

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.

Yes, but that's the internal ID, not the filename

uuid = id.substring("BQ-JDBC-".length());
}
String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
String shortUuid = uuid.length() >= 4 ? uuid.substring(0, 4) : uuid;
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.

nit: no need for that check, uuid will never be shorter than 4

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