Skip to content

src: add tracing category macros #19155

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.

Sign up for GitHub

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

Jump to bottom
Closed
jasnell wants to merge 2 commits into from
Closed

src: add tracing category macros #19155

jasnell wants to merge 2 commits into from

Conversation

@jasnell
Copy link
Member

@jasnell jasnell commented Mar 5, 2018

Adds TRACING_CATEGORY_NODE, TRACING_CATEGORY_NODE1 and
TRACING_CATEGORY_NODE2 helper macros for consistently building
trace event category strings. For instance,

TRACING_CATEGORY_NODE2(foo, bar) would generate the category
string node,node.foo,node.foo.bar, such that...

TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(
    TRACING_CATEGORY_NODE2(foo, bar),
    "baz", 1);

Would emit if trace events are enabled for categories: node,
node.foo, or node.foo.bar.

This allows a natural scoping down of what trace events a user
may want to receive. Enabling the node category would receive
everything Node.js produces.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@jasnell jasnell added async_hooks Issues and PRs related to the async hooks subsystem. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events. perf_hooks Issues and PRs related to the implementation of the Performance Timing API. labels Mar 5, 2018
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Mar 5, 2018
@jasnell
Copy link
Member Author

jasnell commented Mar 5, 2018

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

@jasnell
src: add tracing category macros
ce65bb0 
Adds `TRACING_CATEGORY_NODE`, `TRACING_CATEGORY_NODE1` and
`TRACING_CATEGORY_NODE2` helper macros for consistently building
trace event category strings. For instance,

`TRACING_CATEGORY_NODE2(foo, bar)` would generate the category
string `node,node.foo,node.foo.bar`, such that...

```
TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(
    TRACING_CATEGORY_NODE2(foo, bar),
    "baz", 1);
```

Would emit if trace events are enabled for categories: `node`,
`node.foo`, or `node.foo.bar`.

This allows a natural scoping down of what trace events a user
may want to receive. Enabling the `node` category would receive
everything Node.js produces.
@jasnell jasnell force-pushed the tracing-category-helper branch from 76181d7 to ce65bb0 Compare March 8, 2018 17:12
@jasnell
Copy link
Member Author

jasnell commented Mar 8, 2018

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

AndreasMadsen
AndreasMadsen approved these changes Mar 8, 2018
View reviewed changes
Copy link
Member

@AndreasMadsen AndreasMadsen left a comment

Choose a reason for hiding this comment

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

LGTM, now we just need V8 to adopt the the same so I can have v8.gc :D

jasnell added a commit that referenced this pull request Mar 9, 2018
@jasnell
src: add tracing category macros
1dd9c97 
Adds `TRACING_CATEGORY_NODE`, `TRACING_CATEGORY_NODE1` and
`TRACING_CATEGORY_NODE2` helper macros for consistently building
trace event category strings. For instance,

`TRACING_CATEGORY_NODE2(foo, bar)` would generate the category
string `node,node.foo,node.foo.bar`, such that...

```
TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(
    TRACING_CATEGORY_NODE2(foo, bar),
    "baz", 1);
```

Would emit if trace events are enabled for categories: `node`,
`node.foo`, or `node.foo.bar`.

This allows a natural scoping down of what trace events a user
may want to receive. Enabling the `node` category would receive
everything Node.js produces.

PR-URL: #19155
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Mar 9, 2018

Landed in 1dd9c97

@jasnell jasnell closed this Mar 9, 2018
@targos
Copy link
Member

targos commented Mar 17, 2018

Should this be backported to v9.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@jasnell @MayaLekova
src: add tracing category macros
a3be450 
Adds `TRACING_CATEGORY_NODE`, `TRACING_CATEGORY_NODE1` and
`TRACING_CATEGORY_NODE2` helper macros for consistently building
trace event category strings. For instance,

`TRACING_CATEGORY_NODE2(foo, bar)` would generate the category
string `node,node.foo,node.foo.bar`, such that...

```
TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(
    TRACING_CATEGORY_NODE2(foo, bar),
    "baz", 1);
```

Would emit if trace events are enabled for categories: `node`,
`node.foo`, or `node.foo.bar`.

This allows a natural scoping down of what trace events a user
may want to receive. Enabling the `node` category would receive
everything Node.js produces.

PR-URL: nodejs#19155
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
@jasnell
Copy link
Member Author

jasnell commented Aug 17, 2018

will open a separate backport PR to get this into 8.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax Awaiting requested review from addaleax

1 more reviewer

@AndreasMadsen AndreasMadsen AndreasMadsen approved these changes

Reviewers whose approvals may not affect merge requirements

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++. perf_hooks Issues and PRs related to the implementation of the Performance Timing API. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jasnell @targos @AndreasMadsen @nodejs-github-bot