GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — trace_events: add executionAsyncId to init events by AndreasMadsen · Pull Request #17196 · nodejs/node

Conversation

AndreasMadsen commented Nov 21, 2017

Copy link

Copy Markdown

Member

async_hooks emits trace_events. This adds the executionAsyncId to the
init events. In theory this could be inferred from the before and after
events but this is much simpler and doesn't require knowledge of all
events.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

async_hooks, trace_events

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label

Nov 21, 2017

AndreasMadsen commented Nov 21, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/11588/

/cc @nodejs/async_hooks @jasnell

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AndreasMadsen added async_hooks

Issues and PRs related to the async hooks subsystem.

async_wrap labels

Nov 21, 2017

jasnell approved these changes

Nov 21, 2017

View reviewed changes


          trace_events: add executionAsyncId to init events

bbaf003

async_hooks emits trace_events. This adds the executionAsyncId to the
init events. In theory this could be inferred from the before and after
events but this is much simpler and doesn't require knowledge of all
events.

AndreasMadsen added dont-land-on-v4.x labels

Nov 21, 2017

fhinkel approved these changes

Nov 21, 2017

View reviewed changes

fhinkel left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM with my comment addressed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/node_trace_events.cc Outdated

		num_args = 0;
		} else {
		if (args.Length() >= 6 &&
		!args[4]->IsUndefined() &&

fhinkel Nov 21, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Was the logic wrong here before? If we're not changing the logic, the negation of the statement should be
if(args.Length() >= 6 && (!args[4]->IsUndefined() || !args[5]->IsUndefined())). DeMorgan changes the second && to a ||.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

AndreasMadsen Nov 21, 2017

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Right. I think it is a matter of opinion. But since we have validation after we should properly go with what you suggest.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          [squash] DeMorgan

6e48ed3

cjihrig approved these changes

Nov 22, 2017

View reviewed changes

AndreasMadsen commented Nov 23, 2017

Copy link

Copy Markdown

Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/11664/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AndreasMadsen commented Nov 24, 2017

Copy link

Copy Markdown

Member Author

Landed in ad80c21

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AndreasMadsen closed this

Nov 24, 2017

MylesBorins pushed a commit that referenced this pull request

Dec 12, 2017


          trace_events: add executionAsyncId to init events

7fd00ac

async_hooks emits trace_events. This adds the executionAsyncId to the
init events. In theory this could be inferred from the before and after
events but this is much simpler and doesn't require knowledge of all
events.

PR-URL: #17196
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

AndreasMadsen added the lts-watch-v8.x label

Dec 12, 2017

MylesBorins pushed a commit that referenced this pull request

Dec 12, 2017


          trace_events: add executionAsyncId to init events

855bb8d

async_hooks emits trace_events. This adds the executionAsyncId to the
init events. In theory this could be inferred from the before and after
events but this is much simpler and doesn't require knowledge of all
events.

PR-URL: #17196
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

MylesBorins mentioned this pull request

Dec 12, 2017

v9.3.0 proposal #17631

Merged

AndreasMadsen added the semver-minor PRs that contain new features and should be released in the next minor version. label

Dec 12, 2017

AndreasMadsen added the backported-to-v9.x label

Jan 8, 2018

This was referenced Jan 16, 2018

Semver Minor Tracking nodejs/Release#298

Closed

Backport async hooks to v8.x #18179

Closed

gibfahn pushed a commit to gibfahn/node that referenced this pull request

Jan 17, 2018


          trace_events: add executionAsyncId to init events

66c6650

async_hooks emits trace_events. This adds the executionAsyncId to the
init events. In theory this could be inferred from the before and after
events but this is much simpler and doesn't require knowledge of all
events.

PR-URL: nodejs#17196
Backport-PR-URL: nodejs#18179
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

MylesBorins pushed a commit that referenced this pull request

Jan 19, 2018


          trace_events: add executionAsyncId to init events

ae4428e

async_hooks emits trace_events. This adds the executionAsyncId to the
init events. In theory this could be inferred from the before and after
events but this is much simpler and doesn't require knowledge of all
events.

Backport-PR-URL: #18179
PR-URL: #17196
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

gibfahn added a commit that referenced this pull request

Mar 6, 2018


          2018-03-06 Version 8.10.0 'Carbon' (LTS)

2fce636

Notable changes:

* deps:
  * update V8 to 6.2.414.46 (Michaël Zasso) [#16413](#16413)
  * revert ABI breaking changes in V8 6.2 (Anna Henningsen) [#16413](#16413)
  * upgrade libuv to 1.19.1 (cjihrig) [#18260](#18260)
  * re land npm 5.6.0 (Myles Borins) [#18625](#18625)
  * ICU 60 bump (Steven R. Loomis) [#16876](#16876)
* crypto:
  * Support both OpenSSL 1.1.0 and 1.0.2 (David Benjamin) [#16130](#16130)
  * warn on invalid authentication tag length (Tobias Nießen) [#17566](#17566)
* async_hooks:
  * update defaultTriggerAsyncIdScope for perf (Anatoli Papirovski) [#18004](#18004)
  * use typed array stack as fast path (Anna Henningsen) [#17780](#17780)
  * use scope for defaultTriggerAsyncId (Andreas Madsen) [#17273](#17273)
  * separate missing from default context (Andreas Madsen) [#17273](#17273)
  * rename initTriggerId (Andreas Madsen) [#17273](#17273)
  * deprecate undocumented API (Andreas Madsen) [#16972](#16972)
  * add destroy event for gced AsyncResources (Sebastian Mayr) [#16998](#16998)
  * add trace events to async_hooks (Andreas Madsen) [#15538](#15538)
  * set HTTPParser trigger to socket (Andreas Madsen) [#18003](#18003)
  * add provider types for net server (Andreas Madsen) [#17157](#17157)
* n-api:
  * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109)
* cli:
  * add --stack-trace-limit to NODE_OPTIONS (Anna Henningsen) [#16495](#16495)
* console:
  * add support for console.debug (Benjamin Zaslavsky) [#17033](#17033)
* module:
  * add builtinModules (Jon Moss) [#16386](#16386)
  * replace default paths in require.resolve() (cjihrig) [#17113](#17113)
* src:
  * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109)
  * add process.ppid (cjihrig) [#16839](#16839)
* http:
  * support generic `Duplex` streams (Anna Henningsen) [#16267](#16267)
  * add rawPacket in err of `clientError` event (XadillaX) [#17672](#17672)
  * better support for IPv6 addresses (Mattias Holmlund) [#14772](#14772)
* net:
  * remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [#17662](#17662)
* process:
  * fix reading zero-length env vars on win32 (Anna Henningsen) [#18463](#18463)
* tls:
  * unconsume stream on destroy (Anna Henningsen) [#17478](#17478)
* process:
  * improve unhandled rejection message (Madara Uchiha) [#17158](#17158)
* stream:
  * remove usage of *State.highWaterMark (Calvin Metcalf) [#12860](#12860)
* trace_events:
  * add executionAsyncId to init events (Andreas Madsen) [#17196](#17196)

PR-URL: #18336

gibfahn added a commit that referenced this pull request

Mar 7, 2018


          2018-03-06 Version 8.10.0 'Carbon' (LTS)

fc9ddd9

Notable changes:

* deps:
  * update V8 to 6.2.414.46 (Michaël Zasso) [#16413](#16413)
  * revert ABI breaking changes in V8 6.2 (Anna Henningsen) [#16413](#16413)
  * upgrade libuv to 1.19.1 (cjihrig) [#18260](#18260)
  * re land npm 5.6.0 (Myles Borins) [#18625](#18625)
  * ICU 60 bump (Steven R. Loomis) [#16876](#16876)
* crypto:
  * Support both OpenSSL 1.1.0 and 1.0.2 (David Benjamin) [#16130](#16130)
  * warn on invalid authentication tag length (Tobias Nießen) [#17566](#17566)
* async_hooks:
  * update defaultTriggerAsyncIdScope for perf (Anatoli Papirovski) [#18004](#18004)
  * use typed array stack as fast path (Anna Henningsen) [#17780](#17780)
  * use scope for defaultTriggerAsyncId (Andreas Madsen) [#17273](#17273)
  * separate missing from default context (Andreas Madsen) [#17273](#17273)
  * rename initTriggerId (Andreas Madsen) [#17273](#17273)
  * deprecate undocumented API (Andreas Madsen) [#16972](#16972)
  * add destroy event for gced AsyncResources (Sebastian Mayr) [#16998](#16998)
  * add trace events to async_hooks (Andreas Madsen) [#15538](#15538)
  * set HTTPParser trigger to socket (Andreas Madsen) [#18003](#18003)
  * add provider types for net server (Andreas Madsen) [#17157](#17157)
* n-api:
  * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109)
* cli:
  * add --stack-trace-limit to NODE_OPTIONS (Anna Henningsen) [#16495](#16495)
* console:
  * add support for console.debug (Benjamin Zaslavsky) [#17033](#17033)
* module:
  * add builtinModules (Jon Moss) [#16386](#16386)
  * replace default paths in require.resolve() (cjihrig) [#17113](#17113)
* src:
  * add helper for addons to get the event loop (Anna Henningsen) [#17109](#17109)
  * add process.ppid (cjihrig) [#16839](#16839)
* http:
  * support generic `Duplex` streams (Anna Henningsen) [#16267](#16267)
  * add rawPacket in err of `clientError` event (XadillaX) [#17672](#17672)
  * better support for IPv6 addresses (Mattias Holmlund) [#14772](#14772)
* net:
  * remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [#17662](#17662)
* process:
  * fix reading zero-length env vars on win32 (Anna Henningsen) [#18463](#18463)
* tls:
  * unconsume stream on destroy (Anna Henningsen) [#17478](#17478)
* process:
  * improve unhandled rejection message (Madara Uchiha) [#17158](#17158)
* stream:
  * remove usage of *State.highWaterMark (Calvin Metcalf) [#12860](#12860)
* trace_events:
  * add executionAsyncId to init events (Andreas Madsen) [#17196](#17196)

PR-URL: #18336

MayaLekova pushed a commit to MayaLekova/node that referenced this pull request

May 8, 2018


          2018-03-06 Version 8.10.0 'Carbon' (LTS)

185ac06

Notable changes:

* deps:
  * update V8 to 6.2.414.46 (Michaël Zasso) [nodejs#16413](nodejs#16413)
  * revert ABI breaking changes in V8 6.2 (Anna Henningsen) [nodejs#16413](nodejs#16413)
  * upgrade libuv to 1.19.1 (cjihrig) [nodejs#18260](nodejs#18260)
  * re land npm 5.6.0 (Myles Borins) [nodejs#18625](nodejs#18625)
  * ICU 60 bump (Steven R. Loomis) [nodejs#16876](nodejs#16876)
* crypto:
  * Support both OpenSSL 1.1.0 and 1.0.2 (David Benjamin) [nodejs#16130](nodejs#16130)
  * warn on invalid authentication tag length (Tobias Nießen) [nodejs#17566](nodejs#17566)
* async_hooks:
  * update defaultTriggerAsyncIdScope for perf (Anatoli Papirovski) [nodejs#18004](nodejs#18004)
  * use typed array stack as fast path (Anna Henningsen) [nodejs#17780](nodejs#17780)
  * use scope for defaultTriggerAsyncId (Andreas Madsen) [nodejs#17273](nodejs#17273)
  * separate missing from default context (Andreas Madsen) [nodejs#17273](nodejs#17273)
  * rename initTriggerId (Andreas Madsen) [nodejs#17273](nodejs#17273)
  * deprecate undocumented API (Andreas Madsen) [nodejs#16972](nodejs#16972)
  * add destroy event for gced AsyncResources (Sebastian Mayr) [nodejs#16998](nodejs#16998)
  * add trace events to async_hooks (Andreas Madsen) [nodejs#15538](nodejs#15538)
  * set HTTPParser trigger to socket (Andreas Madsen) [nodejs#18003](nodejs#18003)
  * add provider types for net server (Andreas Madsen) [nodejs#17157](nodejs#17157)
* n-api:
  * add helper for addons to get the event loop (Anna Henningsen) [nodejs#17109](nodejs#17109)
* cli:
  * add --stack-trace-limit to NODE_OPTIONS (Anna Henningsen) [nodejs#16495](nodejs#16495)
* console:
  * add support for console.debug (Benjamin Zaslavsky) [nodejs#17033](nodejs#17033)
* module:
  * add builtinModules (Jon Moss) [nodejs#16386](nodejs#16386)
  * replace default paths in require.resolve() (cjihrig) [nodejs#17113](nodejs#17113)
* src:
  * add helper for addons to get the event loop (Anna Henningsen) [nodejs#17109](nodejs#17109)
  * add process.ppid (cjihrig) [nodejs#16839](nodejs#16839)
* http:
  * support generic `Duplex` streams (Anna Henningsen) [nodejs#16267](nodejs#16267)
  * add rawPacket in err of `clientError` event (XadillaX) [nodejs#17672](nodejs#17672)
  * better support for IPv6 addresses (Mattias Holmlund) [nodejs#14772](nodejs#14772)
* net:
  * remove ADDRCONFIG DNS hint on Windows (Bartosz Sosnowski) [nodejs#17662](nodejs#17662)
* process:
  * fix reading zero-length env vars on win32 (Anna Henningsen) [nodejs#18463](nodejs#18463)
* tls:
  * unconsume stream on destroy (Anna Henningsen) [nodejs#17478](nodejs#17478)
* process:
  * improve unhandled rejection message (Madara Uchiha) [nodejs#17158](nodejs#17158)
* stream:
  * remove usage of *State.highWaterMark (Calvin Metcalf) [nodejs#12860](nodejs#12860)
* trace_events:
  * add executionAsyncId to init events (Andreas Madsen) [nodejs#17196](nodejs#17196)

PR-URL: nodejs#18336

Reviewers

jasnell

jasnell approved these changes

cjihrig

cjihrig approved these changes

+1 more reviewer

fhinkel

fhinkel approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

async_hooks

Issues and PRs related to the async hooks subsystem.

c++

Issues and PRs that require attention from people who are familiar with C++.

semver-minor

PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

AndreasMadsen commented Nov 21, 2017

Checklist

Affected core subsystem(s)

Uh oh!

AndreasMadsen commented Nov 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fhinkel left a comment

Choose a reason for hiding this comment

Uh oh!

fhinkel Nov 21, 2017

Choose a reason for hiding this comment

Uh oh!

AndreasMadsen Nov 21, 2017

Choose a reason for hiding this comment

Uh oh!

AndreasMadsen commented Nov 23, 2017

Uh oh!

AndreasMadsen commented Nov 24, 2017

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AndreasMadsen commented Nov 21, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.