-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
Node.js Added documentation and cross-link for Azure site extension #18896
Node.js Added documentation and cross-link for Azure site extension #18896
Conversation
Hi @mrickard 👋 Thanks for your pull request! Your PR is in a queue, and a writer will take a look soon. We generally publish small edits within one business day, and larger edits within three days. We will automatically generate a preview of your request, and will comment with a link when the preview is ready (usually 10 to 20 minutes). |
✅ Deploy Preview for docs-website-netlify ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Hi @mrickard -- I'm moving this to our drafts column. Since it's a whole new install path, it may take at least three days or up to a sprint to review and complete, depending on the amount of validation and editing that needs to be done :) Please reach out to the hero when you're ready for review!! |
@akristen What would be the best debugging path to follow for those build/deploy failures? It looks as though things have changed significantly in this repo since I last submitted a PR. (Which is understandable!) |
Signed-off-by: mrickard <[email protected]>
9e595d9
to
fd3f776
Compare
Signed-off-by: mrickard <[email protected]>
The Vale linter seems to have a complaint or a failure here; I don't seem to be able to run it locally to understand what changes to make. |
I'll take a look! Our vale linter has been a little neurotic lately so it shouldn't be a blocker. |
@akristen Thank you! I'm also going to add a few changes to some related documents. Would it make more sense to do that as a separate PR, or load them in this one? |
Feel free to keep it all in this PR! (Or if separate is easier for you, that's fine too -- just be sure to leave a comment that links to this PR so we know it's related/connected.) I'm going to do a review right now for what's already here. |
Signed-off-by: mrickard <[email protected]>
Signed-off-by: mrickard <[email protected]>
Signed-off-by: mrickard <[email protected]>
Signed-off-by: mrickard <[email protected]>
src/content/docs/apm/agents/nodejs-agent/getting-started/monitor-your-nodejs-app.mdx
Outdated
Show resolved
Hide resolved
src/content/docs/apm/agents/nodejs-agent/hosting-services/nodejs-agent-microsoft-azure.mdx
Outdated
Show resolved
Hide resolved
src/content/docs/apm/agents/nodejs-agent/hosting-services/nodejs-agent-microsoft-azure.mdx
Outdated
Show resolved
Hide resolved
src/content/docs/apm/agents/nodejs-agent/hosting-services/nodejs-agent-microsoft-azure.mdx
Outdated
Show resolved
Hide resolved
src/content/docs/apm/agents/nodejs-agent/hosting-services/nodejs-agent-microsoft-azure.mdx
Outdated
Show resolved
Hide resolved
src/content/docs/apm/agents/nodejs-agent/hosting-services/nodejs-agent-microsoft-azure.mdx
Outdated
Show resolved
Hide resolved
|
||
The New Relic Node agent is configured with the `newrelic.js` file, or via environment variables. [See our documentation for more detailed configuration](https://docs.newrelic.com/docs/apm/agents/nodejs-agent/installation-configuration/nodejs-agent-configuration/). | ||
|
||
Once the site extension is installed, you'll need to manually enter one configuration item before restarting your application. |
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.
Lines 48-50 is hard to parse. Typically we want to reserve bulleted lists with child (and grand child lists) to follow this kind of pattern:
Sentence goes here:
- Child item 1
- Child item 2
- gchild item 1
- gchild item 2
Rather than a bunch of single bullets after single bullets. I think we can both make it readable and remove the lists so it looks something like this:
Once the site extension is installed, you'll need to manually enter one configuration item before restarting your application. From the Azure portal on the options listed on the left, find Settings and scroll down to Environment variables. Add the
NEW_RELIC_LICENSE_KEY
variable with your license key value.The Node agent automatically adds the
NODE_OPTIONS
environment variable with a value of-r newrelic
. This key-value pair starts the agent. Any previously definedNODE_OPTIONS
will be removed and reset with -r newrelic.
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.
I definitely prefer the rewrite, though there's a slight tweak for accuracy that I'll add in the next commit.
src/content/docs/apm/agents/nodejs-agent/hosting-services/nodejs-agent-microsoft-azure.mdx
Outdated
Show resolved
Hide resolved
...tent/docs/apm/agents/nodejs-agent/installation-configuration/install-nodejs-agent-docker.mdx
Outdated
Show resolved
Hide resolved
…ation/install-nodejs-agent-docker.mdx Co-authored-by: alexa <[email protected]>
…ation/install-nodejs-agent-docker.mdx Co-authored-by: alexa <[email protected]>
Thank you, @akristen ! I'm updating, and I'm also reversing my earlier consolidation: the site extension should really be its own page, crosslinked from the Azure hosting page, since it simplifies installation there. Updates incoming. |
…d edits to text Signed-off-by: mrickard <[email protected]>
src/content/docs/apm/agents/nodejs-agent/getting-started/monitor-your-nodejs-app.mdx
Outdated
Show resolved
Hide resolved
src/content/docs/apm/agents/nodejs-agent/hosting-services/nodejs-agent-microsoft-azure.mdx
Outdated
Show resolved
Hide resolved
src/content/docs/apm/agents/nodejs-agent/hosting-services/nodejs-agent-microsoft-azure.mdx
Outdated
Show resolved
Hide resolved
…or-your-nodejs-app.mdx Co-authored-by: alexa <[email protected]>
src/content/docs/apm/agents/nodejs-agent/hosting-services/nodejs-agent-microsoft-azure.mdx
Outdated
Show resolved
Hide resolved
…js-agent-microsoft-azure.mdx Co-authored-by: alexa <[email protected]>
...agents/nodejs-agent/installation-configuration/install-nodejs-agent-azure-site-extension.mdx
Outdated
Show resolved
Hide resolved
…js-agent-microsoft-azure.mdx Co-authored-by: alexa <[email protected]>
...agents/nodejs-agent/installation-configuration/install-nodejs-agent-azure-site-extension.mdx
Outdated
Show resolved
Hide resolved
...agents/nodejs-agent/installation-configuration/install-nodejs-agent-azure-site-extension.mdx
Show resolved
Hide resolved
...tent/docs/apm/agents/nodejs-agent/installation-configuration/install-nodejs-agent-docker.mdx
Outdated
Show resolved
Hide resolved
...tent/docs/apm/agents/nodejs-agent/installation-configuration/install-nodejs-agent-docker.mdx
Show resolved
Hide resolved
...tent/docs/apm/agents/nodejs-agent/installation-configuration/install-nodejs-agent-docker.mdx
Outdated
Show resolved
Hide resolved
@@ -27,17 +29,19 @@ With just a few additions your existing Dockerfile can be used with our Node.js | |||
"newrelic": "latest", |
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.
Is there a space between the 1 bullet and the snippet?
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.
There should be a line break, so the snippet gets rendered as a code block.
@@ -27,17 +29,19 @@ With just a few additions your existing Dockerfile can be used with our Node.js | |||
"newrelic": "latest", | |||
``` | |||
|
|||
Install a specific version, or use any of the other options provided by the [`package.json` format](https://docs.npmjs.com/files/package.json#dependencies). Check [the Node.js agent release notes](/docs/release-notes/agent-release-notes/nodejs-release-notes) for information about past agent versions. | |||
You can update the value `latest` to install a specific version, or use any of the other options provided by the [`package.json` format](https://docs.npmjs.com/files/package.json#dependencies). Check [the Node.js agent release notes](/docs/release-notes/agent-release-notes/nodejs-release-notes) for information about past agent versions. |
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.
In the deploy preview, this section of text isn't flush as a child under the list. I'd shift tab both the code snippet and this piece of copy so it's nested underneath step 1.
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.
Ah, I wasn't sure how that got rendered. Let me try with my local copy...
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.
Finally fixed it! OK, hang on for a new commit.
…js-agent-microsoft-azure.mdx Co-authored-by: alexa <[email protected]>
Install a specific version, or use any of the other options provided by the [`package.json` format](https://docs.npmjs.com/files/package.json#dependencies). Check [the Node.js agent release notes](/docs/release-notes/agent-release-notes/nodejs-release-notes) for information about past agent versions. | ||
You can update the value `latest` to install a specific version, or use any of the other options provided by the [`package.json` format](https://docs.npmjs.com/files/package.json#dependencies). Check [the Node.js agent release notes](/docs/release-notes/agent-release-notes/nodejs-release-notes) for information about past agent versions. | ||
|
||
2. Instrumentation is most reliable if the New Relic agent is required before your application starts. Your container setup may allow you to edit the `ENTRYPOINT` to include `newrelic` module first with the Node.js `-r`/`--require` flag when the node command is invoked. If your Dockerfile contains any of these start commands, you can change them this way: |
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.
It's best to lead procedures with a clear, direct action that needs to happen. If conceptual info is necessary to help people understand how to do it or why they're doing it, we should add that after the fact. This allows our power users to get to the meat of the procedure while prociding context for newer/less experienced users (that power users can skim past) For example, in step 1, we clearly tell them to "Add X to Y".
What is the action in its most distilled form here? Some suggestions:
- Add the Node.js ``-r
/
--require` flag to the `ENTRYPOINT` so the `newrelic` module appears first. Instrumentation is most reliable if the New Relic agent is required before your app starts. If your container setup doesn't allow this action, then try some of these alternatives:- something
- something
- something
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.
I have a proposed change I'll push in the next commit.
@@ -53,23 +57,27 @@ With just a few additions your existing Dockerfile can be used with our Node.js | |||
|
|||
## Other configuration options [#configure] | |||
|
|||
<Callout variant="caution"> | |||
Don't include your license key in your Dockerfile or Docker image. For more information, see [our documentation on license key security](/docs/accounts/install-new-relic/account-setup/license-key#license-key-security). | |||
<Callout variant="important"> |
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.
Can this be part of a separate h2 section at the start of the doc about versioning, instead of a callout? What's the benefit of it being a callout?
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.
This was a callout early on in this document probably because it's a warning to avoid exposing the license key in committed code.
…ation/install-nodejs-agent-azure-site-extension.mdx Co-authored-by: alexa <[email protected]>
…ation/install-nodejs-agent-azure-site-extension.mdx Co-authored-by: alexa <[email protected]>
…ation/install-nodejs-agent-docker.mdx Co-authored-by: alexa <[email protected]>
Signed-off-by: mrickard <[email protected]>
…ation/install-nodejs-agent-docker.mdx
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.
LET'S GO! :)
Give us some context