-
Notifications
You must be signed in to change notification settings - Fork 245
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
feat: Implement PDP/DataProxy tracing protocol in the binary engine #3505
Conversation
Status update: I solved the problem of some query logs not being captured. I did this by flushing the global tracer at the end of the request, before grabbing the logs for rendering. There's a tradeoff tho' We are using more than one tracing layer, 1) so far the one in charge of capturing logs in memory and returning them in the result. And 2) another one to output logs to STDOUT. The latter flushes spans by default in batches of 512. However, an average request generates less than that (I don't have metrics to assess how much less, and it actually depends a lot on distribution). This means that we are flushing each N < 512 traces, which can lead to an increased frequency in flushing to the STDOUT. I don't think that's super important right now, but might be something to take into account when running this in PDP. The reason why I did not flush the different SpanExporters selectively is that those are hidden abstractions I don't have access to once the pipeline is built. The With this, the implementation is ready for @aqrln and @danstarns to implement front-end changes. I will continue by
Which I hope to finish tomorrow / friday |
5bfa149
to
ac57c9d
Compare
cbcaa14
to
24e3702
Compare
6b27b32
to
a6032cf
Compare
488c03a
to
0f4ba10
Compare
012a890
to
9ff2b46
Compare
afa1bf2
to
7ca3af9
Compare
0a2c2f1
to
f79549f
Compare
Status udpate: The scope of this PR is frozen to implement the capture protocol as described in the contract spec This is to let the prisma/prisma proceed with the integration. Further improvements are carried on in a separate PR: #3549 |
Given I didn't have a review on this PR and that the diff of #3549 was almost neutral. I proceed to merge improvements into this branch, and that way, let reviewers to only review one PR. |
18a15a1
to
94aa3ad
Compare
This commit is limited to sql-query-connector, it should not interfere with #3505. In general, we do not need ownership of the trace id in sql-query-connector, since we only re-render it in comments. Working with a reference is easier (it ii `Copy`, etc.), and it already saves us string copies and allocations with this commit. The other purpose of this PR is that we discussed yesterday about introducing a `QueryContext<'_>` type struct to hold things like the trace id and connection info. Experience from designing similar context structs in the schema team showed it is much easier to do if everything in the context can be a reference. On the side, I could not resist making a few public items crate-public, to make the public surface of the crate smaller and clearer.
This commit is limited to sql-query-connector, it should not interfere with #3505. In general, we do not need ownership of the trace id in sql-query-connector, since we only re-render it in comments. Working with a reference is easier (it ii `Copy`, etc.), and it already saves us string copies and allocations with this commit. The other purpose of this PR is that we discussed yesterday about introducing a `QueryContext<'_>` type struct to hold things like the trace id and connection info. Experience from designing similar context structs in the schema team showed it is much easier to do if everything in the context can be a reference. On the side, I could not resist making a few public items crate-public, to make the public surface of the crate smaller and clearer.
This commit is limited to sql-query-connector, it should not interfere with #3505. In general, we do not need ownership of the trace id in sql-query-connector, since we only re-render it in comments. Working with a reference is easier (it ii `Copy`, etc.), and it already saves us string copies and allocations with this commit. The other purpose of this PR is that we discussed yesterday about introducing a `QueryContext<'_>` type struct to hold things like the trace id and connection info. Experience from designing similar context structs in the schema team showed it is much easier to do if everything in the context can be a reference. On the side, I could not resist making a few public items crate-public, to make the public surface of the crate smaller and clearer.
40d3dd8
to
70023df
Compare
...ory/query-engine/connector-test-kit-rs/query-tests-setup/src/runner/binary_20230116193406.rs
Outdated
Show resolved
Hide resolved
c2dd630
to
2fc3873
Compare
From 423cba4 I think logs are working on the server but not properly handled by either mini-proxy or the client: https://github.com/prisma/prisma/pull/16748/files#r1081177632 |
See #3618 instead
Supersedes #3112
Tracked in https://github.com/prisma/client-planning/issues/158
Sibling to prisma/prisma#16748
To be released after the prep refactoring in #3616
Closes https://github.com/prisma/client-planning/issues/138
Closes https://github.com/prisma/client-planning/issues/144
Closes https://github.com/prisma/client-planning/issues/145
Implements traces and log capturing in concordance with the (private link) contract specification
The technical design is best explained throw the interaction diagram below:
In the diagram, you will see objects whose lifetime is static. The boxes for those have a double
width margin. These are:
server
itselfTRACER
, which handleslog!
andspan!
and uses the globalPROCESSOR
toprocess the data constituting a trace
Span
s and logEvent
sPROCESSOR
, which manages theStorage
set of data structures, holding logs,traces (and capture settings) per request.
Then, through the request lifecycle, different objects are created and dropped:
Settings
] object is built, thisobject determines, for the request, how logging and tracing are going to be captured: if only
traces, logs, or both, and which log levels are going to be captured.
Capturer
is created; a capturer is nothing but an exporterwrapped to start capturing / fetch the captures for this particular request.
Then the capturing process works in this way:
Capture
object [2], which is configured with the settingsdenoted by the
X-capture-telemetry
Capturer
to start capturing all the logs and traces occurring onthe request [3] (denoted by a
trace_id
) Thetrace_id
is either carried on thetraceparent
header or implicitly created on the first span of the request. To start capturing implies
creating for the
trace_id
in two different data structures:logs
andtraces
; and storingthe settings for selecting the Spans and Event to capture [4].
TRACER
, and get hydrated in thePROCESSOR
[7],which writes them in the shard corresponding to the current
trace_id
, into thelogs
andtraces
storage [8]. The settings previously stored under thetrace_id
key, are used to pick which events and spans are going to be captured based on their level.
PrismaResponse
to theserver [9].
PROCESSOR
to fetch the captures [10]Storage
[11]. At that time, althoughthat's not represented in the diagram, the captures are deleted from the storage, thus
freeing any memory used for capturing during the request
logs
andtraces
extensions in thePrismaResponse
[12],it serializes the extended response in json format and returns it as an HTTP Response
blob [13].
Sample use cases
Server configuration:
LOG_QUERIES=y QE_LOG_LEVEL=TRACE query-engine --enable-telemetry-in-response
that log level determines trace spans and events up to that level of specificity will be generated. If that was e.g.INFO
, trace or debug span events won't appear in the response.Requested both query logs, and tracing events:
Response
Requested only query logs:
Response
Requested only traces:
Response