Conversation
rochdev
force-pushed
the
writer
branch
2 times, most recently
from
January 26, 2018 23:46
97ceb96 to
2a277ce
Compare
vinvol
approved these changes
Jan 29, 2018
alrosot
pushed a commit
to alrosot/dd-trace-js
that referenced
this pull request
May 20, 2021
Don't use browserstack
6 tasks
crysmags
added a commit
that referenced
this pull request
May 14, 2026
After merging master, #8309's perf wins target functions that either no longer exist (wrapExecute / wrapFields / pathToArray / finishResolvers moved out of instrumentations/graphql.js) or live in a deleted file (resolve.js was folded into execute.js). Port the still-applicable changes onto the migrated execute plugin: #1 _startResolveSpan uses info.fieldNodes[0] instead of .find(fn => fn.kind === 'Field'). The executor only hands FieldNode siblings. #2 pathToArray switches to count-then-fill (drops the trailing .reverse() and the push-grow). #3 getResolverInfo lazy-allocates resolverVars: skips both the up-front {} and the directive-loop assignment when args is empty and no directives have arguments. #4 shouldInstrument early-returns on config.depth < 0 (no-limit, the default-enabled case) before walking the path. #5 buildCollapsedPathString builds the collapsed key in one walk; drops the intermediate collapsedPath array. iastResolveCh.publish now passes raw path / pathString, matching the contract the old resolve.js had with the IAST subscriber. drive-by wrapFields iterates Object.values(type._fields) directly. #7 (addVariableTags early-return) already auto-merged during the master merge. #6 (single documentSources.get in wrapExecute) is already the shape of bindStart in the migrated plugin. The reverted depth=0 bindStart short-circuit (156a557 / ca2d041) sat at a different position — inside bindStart, gated on _depthDisabled (config.depth === 0). It was rolled back because piling it on top of the resolveAsync depth-flag fix didn't move plugin-graphql-long-with- depth-off any further. This port stays inside shouldInstrument, fires on config.depth < 0 (the *enabled* default), and targets the hot path on plugin-graphql-long. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
crysmags
added a commit
that referenced
this pull request
May 19, 2026
After merging master, #8309's perf wins target functions that either no longer exist (wrapExecute / wrapFields / pathToArray / finishResolvers moved out of instrumentations/graphql.js) or live in a deleted file (resolve.js was folded into execute.js). Port the still-applicable changes onto the migrated execute plugin: #1 _startResolveSpan uses info.fieldNodes[0] instead of .find(fn => fn.kind === 'Field'). The executor only hands FieldNode siblings. #2 pathToArray switches to count-then-fill (drops the trailing .reverse() and the push-grow). #3 getResolverInfo lazy-allocates resolverVars: skips both the up-front {} and the directive-loop assignment when args is empty and no directives have arguments. #4 shouldInstrument early-returns on config.depth < 0 (no-limit, the default-enabled case) before walking the path. #5 buildCollapsedPathString builds the collapsed key in one walk; drops the intermediate collapsedPath array. iastResolveCh.publish now passes raw path / pathString, matching the contract the old resolve.js had with the IAST subscriber. drive-by wrapFields iterates Object.values(type._fields) directly. #7 (addVariableTags early-return) already auto-merged during the master merge. #6 (single documentSources.get in wrapExecute) is already the shape of bindStart in the migrated plugin. The reverted depth=0 bindStart short-circuit (156a557 / ca2d041) sat at a different position — inside bindStart, gated on _depthDisabled (config.depth === 0). It was rolled back because piling it on top of the resolveAsync depth-flag fix didn't move plugin-graphql-long-with- depth-off any further. This port stays inside shouldInstrument, fires on config.depth < 0 (the *enabled* default), and targets the hot path on plugin-graphql-long. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
christophe-papazian
added a commit
that referenced
this pull request
Jun 16, 2026
- #3 (regression from round 5): constraintMatchesSlash no longer treats a '/' that appears only inside a character class as slash-spanning. `[^/]+` denies slashes, so /:id([^/]+)/users (Express 4) normalizes to /{id}/users again instead of null. - #2: a non-terminal catch-all (a wildcard with a non-empty segment after it, incl. an optional {/*rest}/tail) is rejected at compile (→ null) rather than silently dropped. - #1: a segment containing two independent optional groups (e.g. :a{.:b}{-:c}) is rejected (→ null); our single per-segment regex can't replicate path-to-regexp's ordered-alternative assignment, so the combined name could be wrong. Single intra-segment optionals (:id{.:format}) are unaffected. - #4: structural-only optional groups — a {...} wrapping only nested group(s) with no segment/token of its own (e.g. the outer braces in /a{{/b}}) — are collapsed by reparenting represented groups to their nearest represented ancestor, so /a{{/b}} resolves to /a/b | /a instead of always /a. Adds a regression test per finding (verified against live Express 5). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a Writer class that acts as the buffer between the tracer and the transport. It also handles encoding every trace to MsgPack. Since the writer also optimizes how traces are encoded as they are received, that's why it also handles encoding.