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

sql,tracing: harmonize span/traceID representation #60668

Closed
tbg opened this issue Feb 17, 2021 · 2 comments
Closed

sql,tracing: harmonize span/traceID representation #60668

tbg opened this issue Feb 17, 2021 · 2 comments
Labels
A-sql-observability Related to observability of the SQL layer A-tracing Relating to tracing in CockroachDB. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) no-issue-activity X-stale

Comments

@tbg
Copy link
Member

tbg commented Feb 17, 2021

Describe the problem

In SQL, traceIDs/spanIDs are currently represented using DInt, which is an int64-backed type. The tracing package uses uint64s. While this isn't a technical issue, it might be confusing to see int64(x) via SQL and uint64(x) in logs.

Expected behavior
SQL and tracing should agree. Either we use BIGINT in SQL or uint64 in tracing. If we do the latter, we have to be a little careful about the migration (we have to stick to uint64 on the wire, in (*Tracer).InjectX/ExtractX).

Additional data / screenshots
Discussed on https://reviewable.io/reviews/cockroachdb/cockroach/59992

Jira issue: CRDB-3163

@tbg tbg added A-sql-observability Related to observability of the SQL layer A-tracing Relating to tracing in CockroachDB. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) labels Feb 17, 2021
@knz
Copy link
Contributor

knz commented Feb 17, 2021

we use BIGINT in SQL

That is not a thing. SQL BIGINT = signed int64

You may mean DECIMAL or BIT, but these come with obnoxious heap allocation overheads.

@jlinder jlinder added the T-kv KV Team label Jun 16, 2021
@mwang1026 mwang1026 removed the T-kv KV Team label Aug 20, 2021
@github-actions
Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-observability Related to observability of the SQL layer A-tracing Relating to tracing in CockroachDB. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) no-issue-activity X-stale
Projects
None yet
Development

No branches or pull requests

4 participants