Skip to content
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

Add Node.js DI documentation page #27150

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add Node.js DI documentation page #27150

wants to merge 3 commits into from
+139 −25

Conversation

watson
Copy link
Contributor

@watson watson commented Jan 16, 2025 •
edited
Loading

What does this PR do? What is the motivation?

  1. Add a Node.js page to the Dynamic Instrumentation (DI) documentation
  2. Replicate minor changes to the shared "template" in the other languages
  3. Add "Limited Preview" callout with link to waiting list to all languages being in Limited Preview (Node.js, PHP and Ruby)
  4. Add "Limitations" section to all languages being in Limited Preview (Node.js, PHP and Ruby)

Merge instructions

Merge readiness:

  • Ready for merge

Merge queue is enabled in this repo. To have it automatically merged after it receives the required reviews, create the PR (from a branch that follows the <yourname>/description naming convention) and then add the following PR comment:

/merge

Additional notes

@watson Graphite App
Copy link
Contributor Author

watson commented Jan 16, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 16, 2025 •
edited
Loading

Preview links (active after the build_preview check completes)

New or renamed files

Modified Files

@watson watson force-pushed the watson/DEBUG-2755/add-nodejs-docs branch 5 times, most recently from cbf951a to 4cb79c7 Compare January 16, 2025 13:32
@watson watson force-pushed the watson/DEBUG-2755/add-nodejs-docs branch 3 times, most recently from 5c79b36 to 5267746 Compare January 17, 2025 12:30
@watson watson force-pushed the watson/DEBUG-2755/add-nodejs-docs branch from e7231f4 to 5267746 Compare January 17, 2025 13:11
@watson watson mentioned this pull request Jan 17, 2025
Closed
1 task
@watson watson force-pushed the watson/DEBUG-2755/add-nodejs-docs branch 2 times, most recently from dc71835 to bd4fba1 Compare January 21, 2025 10:03
@github-actions github-actions bot added the Architecture Everything related to the Doc backend label Jan 21, 2025
@watson watson force-pushed the watson/DEBUG-2755/add-nodejs-docs branch 7 times, most recently from 9e03d62 to 0e3958d Compare January 21, 2025 12:13
@watson watson requested review from OmerRaviv and shatzi January 22, 2025 09:37
shatzi
shatzi approved these changes Jan 22, 2025
View reviewed changes
Copy link
Contributor

@shatzi shatzi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think this great to idea to put them in shortcodes and reuse them.

What I don't like about this is our docs become scary. the only configurations DI provide is:

  1. enable.
  2. deal with PII

before we had service,env,version to easy the user in.. I think the configuration section on our own pages should also include the unified ones.

Do we also want to add SYM_DB config ? we tell them to go read the docs but I think better to also include them if they miss the line. maybe even add "RECOMMENDED" tag next to those settings.

Maybe divide those configurations into sections:

  1. enablement - in currently also have symdb and exception_reply. in the future we would add code origin and debugger - lets make a section about what you active.
  2. unified tagging
  3. rate_limit and general configuration - exception reply have some, source code have some..
  4. PII - the scary stuff last.

content/en/dynamic_instrumentation/enabling/java.md Show resolved Hide resolved
content/en/dynamic_instrumentation/enabling/nodejs.md Show resolved Hide resolved
@watson watson force-pushed the watson/DEBUG-2755/add-nodejs-docs branch from 8c38596 to c2434d9 Compare January 23, 2025 07:58
@github-actions github-actions bot removed the Architecture Everything related to the Doc backend label Jan 23, 2025
@watson
Copy link
Contributor Author

watson commented Jan 23, 2025

I've reverted a lot of the changes in this PR as it was getting quite large and hence harder to discuss. I'll open smaller follow up PRs with these changes to make the discussion easier. Thank y'all for all your feedback so far ❤️

@watson watson force-pushed the watson/DEBUG-2755/add-nodejs-docs branch from c2434d9 to f6276c2 Compare January 23, 2025 08:19
OmerRaviv
OmerRaviv approved these changes Jan 23, 2025
View reviewed changes
Copy link
Contributor

@OmerRaviv OmerRaviv left a comment

Choose a reason for hiding this comment

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

LGTM

@watson watson force-pushed the watson/DEBUG-2755/add-nodejs-docs branch from f6276c2 to e4d2e64 Compare January 23, 2025 15:29
@watson watson force-pushed the watson/DEBUG-2755/add-nodejs-docs branch from e4d2e64 to f0b1141 Compare January 23, 2025 16:26
watson
watson commented Jan 28, 2025
View reviewed changes
content/en/dynamic_instrumentation/enabling/nodejs.md
text: 'Getting Started with Datadog Agent'
---

{{< beta-callout-private url="https://forms.gle/TODO" >}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding link once Jira ticket WEB-5882 is done

watson
watson commented Jan 28, 2025
View reviewed changes
content/en/dynamic_instrumentation/enabling/ruby.md
@@ -12,17 +12,22 @@ further_reading:
text: 'Getting Started with Datadog Agent'
---

Dynamic Instrumentation is a feature of supporting Datadog tracing libraries. If you are already using [APM to collect traces][1] for your application, ensure your Agent and tracing library are on the required version, and go directly to enabling Dynamic Instrumentation in step 4.
{{< beta-callout-private url="https://forms.gle/TODO" >}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding link once Jira ticket WEB-5882 is done

@watson watson marked this pull request as ready for review January 28, 2025 14:46
@watson watson requested a review from a team as a code owner January 28, 2025 14:46
brett0000FF
brett0000FF approved these changes Jan 28, 2025
View reviewed changes
Copy link
Contributor

@brett0000FF brett0000FF left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the updates!

Approving contingent on updating the preview form URL before merging.

content/en/dynamic_instrumentation/enabling/nodejs.md
---
title: Enable Dynamic Instrumentation for Node.js
aliases:
- /tracing/dynamic_instrumentation/enabling/nodejs/
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- /tracing/dynamic_instrumentation/enabling/nodejs/

Not needed since this is a new page, and it never existed at this location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't I also remove the aliases: line above?

content/en/dynamic_instrumentation/enabling/php.md
Comment on lines +49 to +52
### Unsupported features

- Dynamic Logs attached to a specific file/line
- Dynamic Metrics, Spans, and Span Tags
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Unsupported features
- Dynamic Logs attached to a specific file/line
- Dynamic Metrics, Spans, and Span Tags

I would suggest removing this section to keep our docs focused on what the product can do. I think this section is ok on the Node.js and Ruby preview pages because it is talking about the preview specifically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have rolled back PHP support and decided to re-release it again soon as a Limited Preview, just as with the Ruby and Node.js tracers. That's why I added this section so it was ready for when we decided to launch.

@OmerRaviv can you verify?

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

@p-datadog p-datadog p-datadog left review comments

@sstonehill12 sstonehill12 sstonehill12 approved these changes

@brett0000FF brett0000FF brett0000FF approved these changes

@shatzi shatzi shatzi approved these changes

@OmerRaviv OmerRaviv OmerRaviv approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@watson @OmerRaviv @shatzi @sstonehill12 @brett0000FF @p-datadog