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.

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

Improve OpenTelemetry entity documentation #17188

Merged
merged 6 commits into from
May 8, 2024

Conversation

alanwest
Copy link
Contributor

@alanwest alanwest commented May 3, 2024

No description provided.

@CLAassistant
Copy link

CLAassistant commented May 3, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

github-actions bot commented May 3, 2024

Hi @alanwest 👋

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).

@github-actions github-actions bot added this to Hero to triage in Docs PRs and Issues May 3, 2024
Copy link

netlify bot commented May 3, 2024

Deploy Preview for docs-website-netlify ready!

Name Link
🔨 Latest commit 0925748
🔍 Latest deploy log https://app.netlify.com/sites/docs-website-netlify/deploys/663a87d81222670008b9bb9b
😎 Deploy Preview https://deploy-preview-17188--docs-website-netlify.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@homelessbirds homelessbirds added content requests related to docs site content from_internal Identifies issues/PRs from Relics (except writers) labels May 3, 2024
@homelessbirds homelessbirds moved this from Hero to triage to In progress in Docs PRs and Issues May 3, 2024
Copy link
Contributor

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

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

Couple comments but looks good


<img width="254" alt="Screen Shot 2022-08-15 at 2 22 34 PM" src="https://user-images.githubusercontent.com/48657837/184720791-d2040326-55e6-4932-a4da-87d5ed4e6801.png"/>
### Services
Copy link
Contributor

Choose a reason for hiding this comment

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

Couple presentation questions:

  • If you want this to show up in the in-page navigation, you need to it to use the same header level as the top level used on the page. So since you use an h2 above, anything lower than h2 like this h3 will be omitted.
  • What do you think about a table to represent this information?
Type Required attributes Recommended attributes Entity tags
service service.name service.instance.id - Enables faceting between multiple instances of the same service.
telemetry.sdk.language - Drives display of language specific runtime UIs, like the JVM runtime page for Java
service.namespace
telemetry.sdk.language
telemetry.sdk.version
k8s.cluster.name
k8s.namespace.name
k8s.deployment.name

The list view allows for more detailed description, but the tabular view increases information density. List is the conservative approach since we may likely find ourselves needing to give longer explanations.

Copy link
Contributor Author

@alanwest alanwest May 3, 2024

Choose a reason for hiding this comment

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

List is the conservative approach since we may likely find ourselves needing to give longer explanations.

A table layout was my first intuition too but settled on a list for now for this reason pretty much... I suspect we will iterate on this a bit more - maybe adding additional entity types - before we're completely done with our holistic refactor of the otel docs.

I do agree the lists explode the length of the page. I thought about collapsable regions like we use elsewhere in the docs, but I hate those things so much.

That said, no strong opinions yet...

Copy link
Contributor Author

@alanwest alanwest May 3, 2024

Choose a reason for hiding this comment

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

If you want this to show up in the in-page navigation

I conceded and did not promote everything to h2. I think the right hand nav is not too terrible as I've organized it.

  • Supported entity types
  • Supported entity relationships
  • Adding custom tags to an entity

@ally-sassman This is a topic Jack and I have discussed.. The right hand nav is a pretty standard pattern in technical documentation. But I think NR's implementation has a lot to be desired.

OpenTelemetry.io is a great example of the pattern applied well.

For example, check out the awesome right hand nav on this page:
https://opentelemetry.io/docs/concepts/signals/traces/

Copy link
Contributor

Choose a reason for hiding this comment

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

The right hand nav is a pretty standard pattern in technical documentation. But I think NR's implementation has a lot to be desired.

I think all it would take would be two levels of headers to be included instead of one.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jack-berg @alanwest I agree that the right nav can and should be improved. I believe our eng team is currently working on the ability to display H3s in the right nav. In the meantime, let's keep writing strong titles for headers keeping the right-nav presentation in mind.

@@ -8,21 +8,117 @@ metaDescription: Here are some tips for working with OpenTelemetry resources and
freshnessValidatedDate: never
---

A resource in OpenTelemetry represents information about an entity generating telemetry data. Make sure all telemetry data sent to New Relic is associated with a resource so it can be linked with the appropriate entity in New Relic. The [OpenTelemetry Resource SDK specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md) defines the functionality implemented by all language SDKs for defining a resource.
All data from OpenTelemetry is associated with a [resource](https://opentelemetry.io/docs/concepts/resources).
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider renaming the name of this page from "OpenTelemetry resources: Best practices". Maybe just "OpenTelemetry Resources"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just left it as is since all the pages that are siblings to this one has the awkward "Best practices" subtitle. May be we just fix them all at once when we get everything moved to where we ultimately want it.

…rations/opentelemetry/best-practices/opentelemetry-best-practices-resources.mdx

Co-authored-by: jack-berg <34418638+jack-berg@users.noreply.github.com>
ally-sassman
ally-sassman previously approved these changes May 7, 2024
…rations/opentelemetry/best-practices/opentelemetry-best-practices-resources.mdx

Co-authored-by: ally sassman <42753584+ally-sassman@users.noreply.github.com>
alanwest and others added 2 commits May 7, 2024 12:57
…rations/opentelemetry/best-practices/opentelemetry-best-practices-resources.mdx

Co-authored-by: ally sassman <42753584+ally-sassman@users.noreply.github.com>
…rations/opentelemetry/best-practices/opentelemetry-best-practices-resources.mdx

Co-authored-by: ally sassman <42753584+ally-sassman@users.noreply.github.com>
@ally-sassman ally-sassman merged commit 5691b0f into newrelic:develop May 8, 2024
12 of 13 checks passed
@alanwest alanwest deleted the alanwest/otel-entities branch May 8, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content requests related to docs site content from_internal Identifies issues/PRs from Relics (except writers)
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants