-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Rework OpenTelemetry logs page #17269
base: develop
Are you sure you want to change the base?
Conversation
Hi @jack-berg 👋 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. |
…to rework-otel-logs
...ce-telemetry-integrations/opentelemetry/best-practices/opentelemetry-best-practices-logs.mdx
Show resolved
Hide resolved
...ce-telemetry-integrations/opentelemetry/best-practices/opentelemetry-best-practices-logs.mdx
Outdated
Show resolved
Hide resolved
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.
@jack-berg See my comment/suggestion, and check out my latest commit
...ce-telemetry-integrations/opentelemetry/best-practices/opentelemetry-best-practices-logs.mdx
Show resolved
Hide resolved
**[2]**: If `LogRecord.time_unix_nanos` is not present, `timestamp` is set to the time that the data was received by New Relic. | ||
|
||
**[3]**: [Log parsing](/docs/logs/ui-data/parsing/) is applied to the `LogRecord.body` to attempt to extract attributes from plain log text. For example, if a JSON structured log format is used, the key / values become attributes on the resulting log. This is particularly useful when collecting logs from files or stdout. In this case, it's common to have no resource attributes associated with the log record (required for [APM service correlation](#service-correlation)) and no value for `LogRecord.trace_id` / `LogRecord.span_id` (required for [trace correlation](#trace-correlation)). Correlation will function as intended if the required fields can be successfully parsed. | ||
New Relic supports two primary workflows for ingesting logs generated with OpenTelemetry: |
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 phrasing isn't correct. These aren't new relic's workflows and we don't explicitly support them. These are the workflows identified / documented by the OpenTelemetry project. At the end of the day, New Relic supports OTLP ingest.
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.
@jack-berg I'm open to changing the phrasing, but I want to keep it New Relic-focused and communicate that we "support" the two OTLP ingest workflows.
How about something like:
- "To send OTLP-ingested logs to New Relic, use one of the following OpenTelemetry workflows:"
- "New Relic supports two OTLP workflows for ingesting OpenTelemetry logs:
I'm open to suggestions.
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.
but I want to keep it New Relic-focused and communicate that we "support" the two OTLP ingest workflows.
But we don't support these. The flows exist, and New Relic simply has an OTLP endpoint, which is the thing we actually support. With that said, I don't want to want to split hairs - I'm fine doing things to keep it New Relic centric. What do you think about:
There are two typical workflows for sending OpenTelemetry logs to New Relic:
New Relic supports two primary workflows for ingesting logs generated with OpenTelemetry: | ||
|
||
* [Direct to collector](https://opentelemetry.io/docs/specs/otel/logs/#direct-to-collector): | ||
* Applications can send logs directly to the dedicated New Relic OpenTelemetry Log Protocol (OTLP) endpoint. This streamlined approach minimizes additional components. |
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.
* Applications can send logs directly to the dedicated New Relic OpenTelemetry Log Protocol (OTLP) endpoint. This streamlined approach minimizes additional components. | |
* Applications can send logs directly to the New Relic OTLP endpoint. |
OTLP != OpenTelemetry Log Protocol endpoint. "This streamlined approach minimizes additional components" repeats content on OpenTelemetry's docs. I think as a principle we need to avoid this because: 1. Less is more. 2. We're not the authority and have a history of getting it wrong. 3. The upstream doc can change and we might be left with a conflicting message.
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.
@jack-berg What do you mean when you say "OTLP != OpenTelemetry Log Protocol endpoint"? How was my version (New Relic OpenTelemetry Log Protocol (OTLP) endpoint) incorrect?
And re: your other point, I added that because it might be helpful to know when/why you'd use one workflow over the other, but we can keep the content more scrubbed down if we're depending on readers going to the OTEL docs.
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.
OTLP is an acronym for OpenTelemetry Protocol, not OpenTelemetry Log Protocol. It was originally an acronym for OpenTelemetry Line Protocol, but "Line" was later dropped after the shorted OTLP had already caught on. OTLP is a general purpose protocol for sending logs, metrics, traces, and profiles.
And re: your other point, I added that because it might be helpful to know when/why you'd use one workflow over the other, but we can keep the content more scrubbed down if we're depending on readers going to the OTEL docs.
Yeah I get it. Its a balance - additional words can add helpful context even if they duplicate content which exists somewhere else, but also add more surface area for mistakes and content drift from the original source. I think in this case, the principle is to focus on "what" is happening with the two workflows, not "why".
|
||
This documentation focuses on how New Relic processes OpenTelemetry logs received through its dedicated OTLP endpoint. Regardless of the chosen collection method, successful integration requires configuring your log source to export logs to this endpoint. Make sure to review the [endpoint configuration requirements](/docs/more-integrations/open-source-telemetry-integrations/opentelemetry/best-practices/opentelemetry-otlp/#configure-endpoint-port-protocol) before proceeding. | ||
|
||
For information on configuring services with OpenTelemetry logs, see [OpenTelemetry APM monitoring](/docs/more-integrations/open-source-telemetry-integrations/opentelemetry/get-started/apm-monitoring/opentelemetry-apm-intro/). |
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.
@jack-berg Let's clarify why they would want to go to the APM intro doc before continuing on this page. Are we saying they should already have APM OTEL monitoring set up, and if not, go see OpenTelemetry APM monitoring?
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.
Yeah good point. I think this link should actually go under the "Direct to collector" bullet point. I want to tell readers that if they want to setup a service with APM log monitoring, this isn't the right page but go look at this other page.
Builds off #17169, #17210.
Rework "OpenTelemetry -> OpenTelemetry data in New Relic -> Logs" page, merging contents of "OpenTelemetry -> Find data in New Relic -> Logs".