FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

etw: fix descriptors of events 9 and 23 by joaocgreis · Pull Request #5742 · nodejs/node · GitHub

/ node Public

etw: fix descriptors of events 9 and 23 - #5742

Closed
joaocgreis wants to merge 2 commits into
nodejs:masterfrom
JaneaSystems:joaocgreis-G3G-fix-etw-9-23
ConversationCommits (2)ChecksFiles changed
Closed

etw: fix descriptors of events 9 and 23#5742
joaocgreis wants to merge 2 commits into
nodejs:masterfrom
JaneaSystems:joaocgreis-G3G-fix-etw-9-23

Conversation

joaocgreis commented Mar 16, 2016

Copy link
Copy Markdown
Member

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to [CONTRIBUTING.md][0]?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?

Affected core subsystem(s)

ETW

Description of change

A log of ETW events can be produced with:

logman start NodejsDCS -p NodeJS-ETW-provider -o node_log.etl -ets
:: Do something with node.exe
logman stop NodejsDCS -ets

The resulting etl file can be open in Event Viewer. However, ETW events 9 and 23 fail to display the EventData (under the Details tab), having instead a ProcessingErrorData section with the data in raw hexadecimal. This happens because the events are not generated correctly.

There are two commits for easy review, I plan to squash and use the following commit message:

etw: fix descriptors of events 9 and 23

Event 9 must include the string terminator in the last descriptor.
Event 23 must be published with no descriptors, in accordance with
the manifest.

cc @nodejs/platform-windows

joaocgreis added the windows Issues and PRs related to the Windows platform. label Mar 16, 2016

jasnell commented Mar 16, 2016

Copy link
Copy Markdown
Member

@bnoordhuis

bnoordhuis commented Mar 19, 2016

Copy link
Copy Markdown
Member

LGTM

jasnell commented Mar 21, 2016

Copy link
Copy Markdown
Member

LGTM if CI is green: https://ci.nodejs.org/job/node-test-pull-request/1991/

jasnell commented Mar 21, 2016

Copy link
Copy Markdown
Member

@joaocgreis is this appropriate for v4?

joaocgreis commented Mar 21, 2016

Copy link
Copy Markdown
Member Author

CI lint fail unrelated, due to #5823.

@jasnell I think this should go in v4. I'll land it in v4.x-staging as well if there's no objection. cc @nodejs/lts

jasnell commented Mar 21, 2016

Copy link
Copy Markdown
Member

@joaocgreis .. SGTM. After you land in v4.x-staging, please remove the lts-watch label and add the land-on-v4 label. Thank you!

MylesBorins commented Mar 21, 2016

Copy link
Copy Markdown
Contributor

@joaocgreis can you hold off on landing on v4.x-staging until after the release of v4.4.1 (which should be tomorrow)

joaocgreis added a commit that referenced this pull request Mar 23, 2016
etw: fix descriptors of events 9 and 23
387b6b4 
Event 9 must include the string terminator in the last descriptor.
Event 23 must be published with no descriptors, in accordance with
the manifest.

PR-URL: #5742
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
joaocgreis added a commit to JaneaSystems/node that referenced this pull request Mar 23, 2016
etw: fix descriptors of events 9 and 23
e7e5af9 
Event 9 must include the string terminator in the last descriptor.
Event 23 must be published with no descriptors, in accordance with
the manifest.

PR-URL: nodejs#5742
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

joaocgreis commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

Landed on master in: 387b6b4

CI for v4.x-staging: https://ci.nodejs.org/job/node-test-commit/2668/

jasnell closed this Mar 23, 2016

jasnell commented Mar 23, 2016

Copy link
Copy Markdown
Member

Closed since it landed in master. Still need to get it landed in v4.x-staging tho. CI looks good. Failure there appears unrelated.

joaocgreis commented Mar 23, 2016

Copy link
Copy Markdown
Member Author

Landed on v4.x-staging in: e7e5af9

Thanks!

MylesBorins commented Mar 30, 2016

Copy link
Copy Markdown
Contributor

I have opted to back this out of v4.x-staging due to the sentiment expressed by the working group in nodejs/Release#90

Will re apply when it has time to mature in v5

jasnell commented Mar 30, 2016

Copy link
Copy Markdown
Member

+1

evanlucas pushed a commit that referenced this pull request Mar 30, 2016
etw: fix descriptors of events 9 and 23
8df627e 
Event 9 must include the string terminator in the last descriptor.
Event 23 must be published with no descriptors, in accordance with
the manifest.

PR-URL: #5742
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas mentioned this pull request Mar 31, 2016
Closed
evanlucas pushed a commit that referenced this pull request Mar 31, 2016
etw: fix descriptors of events 9 and 23
731f7b8 
Event 9 must include the string terminator in the last descriptor.
Event 23 must be published with no descriptors, in accordance with
the manifest.

PR-URL: #5742
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 8, 2016
etw: fix descriptors of events 9 and 23
6d3822c 
Event 9 must include the string terminator in the last descriptor.
Event 23 must be published with no descriptors, in accordance with
the manifest.

PR-URL: #5742
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins commented Apr 8, 2016

Copy link
Copy Markdown
Contributor

This is now back in v4.x-staging

MylesBorins mentioned this pull request Apr 11, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Apr 11, 2016
2016-04-11, Version 4.4.3 'Argon' (LTS)
d555f54 
Notable Changes:

deps:
  - Fix `--gdbjit` for embedders. Backported from v8 upstream.
    (Ben Noordhuis) #5577

etw:
  - Correctly display descriptors for ETW events 9 and 23 on the
    windows platform.
    (João Reis) #5742

querystring:
  - Restore throw when attempting to stringify bad surrogate pair.
    (Brian White) #5858
MylesBorins pushed a commit that referenced this pull request Apr 12, 2016
2016-04-12, Version 4.4.3 'Argon' (LTS)
d7f2bc9 
Notable Changes:

deps:
  - Fix `--gdbjit` for embedders. Backported from v8 upstream.
    (Ben Noordhuis) #5577

etw:
  - Correctly display descriptors for ETW events 9 and 23 on the
    windows platform.
    (João Reis) #5742

querystring:
  - Restore throw when attempting to stringify bad surrogate pair.
    (Brian White) #5858
lukekarrys mentioned this pull request Nov 1, 2022
Closed
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

windows Issues and PRs related to the Windows platform.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL