Documentation for PR #797 in demo app - Collector Data Flow Dashboard

[devrimdemiroz]

Documentation for open-telemetry/opentelemetry-demo#797 on opentelemetry demo application repo.

---

Preview: https://deploy-preview-2538--opentelemetry.netlify.app/docs/demo/collector-data-flow-dashboard/

[cartermp]

Overall LGTM, could you use code fences for the names of the metrics listed here?

[devrimdemiroz]

Overall LGTM, could you use code fences for the names of the metrics listed here?

fenced

[cartermp]

This should fix the link issues.

[chalin]

Currently the newly added page doesn't render (so we can't preview it) because it has no Hugo front matter. I can suggest some front-matter content later if you'd like, but my first question is, where is the new page meant to fit in the overall demo docs? IMHO, we don't want it at the top level.

@open-telemetry/demo-approvers have any comment about this?

The newly created images shouldn't be in the screenshots folder unless they show up in the Screenshots page.

[chalin]

I've added minimal front matter to the added page so that it will at least render. See the opening comment for a preview link.

[chalin]

macro-level --> high-level?

[devrimdemiroz]

I used macro-level on purpose as later when you have many collectors in place, the board can provide macro-level visibility. As per the demo, I agree, it is just high-level. Shall I change?

[chalin]

Is "macro-level" a domain-specific term? If not, I'd vote for "high-level".
@svrnm @cartermp @open-telemetry/demo-approvers - any comments about this?

[cartermp]

I don't mind either way. It's easy enough to change later if we need to.

[devrimdemiroz]

Currently the newly added page doesn't render (so we can't preview it) because it has no Hugo front matter. I can suggest some front-matter content later if you'd like, but my first question is, where is the new page meant to fit in the overall demo docs? IMHO, we don't want it at the top level.

@open-telemetry/demo-approvers have any comment about this?

The newly created images shouldn't be in the screenshots folder unless they show up in the Screenshots page.

I did not find a better place inside this directory. I am happy to receive a recommendation on this one.

[chalin]

I did not find a better place inside this directory.

No worries, that's fine. Waiting on @open-telemetry/demo-approvers and others to weigh in.

[devrimdemiroz]

@chalin, I appreciate your valuable feedback for improvement. I believe my most recent commit addresses the issues. Could you please take another look?

[chalin]

Thanks for the updates @devrimdemiroz. We're getting closer!

I've unresolved some of my comments since they haven't been addressed. Please avoid marking a comment as resolved unless the required changes have landed in the PR. If there are questions or doubts, leave it unresolved so that the comment author or maintainers can make the assessment and update the status as required or follow up with more comments.

The main issue that's left is image placement. We generally want to avoid starting a section with an image. In all cases in the page, the opening paragraph is short enough that the image can be placed after the opening paragraph. Please make that change for all images, and adjust the paragraph text to refer to the "image below" rather than "above.

I'll have a few final copyedits to share after that. Thanks!

[devrimdemiroz]

Thanks for the updates @devrimdemiroz. We're getting closer!

I've unresolved some of my comments since they haven't been addressed. Please avoid marking a comment as resolved unless the required changes have landed in the PR. If there are questions or doubts, leave it unresolved so that the comment author or maintainers can make the assessment and update the status as required or follow up with more comments.

The main issue that's left is image placement. We generally want to avoid starting a section with an image. In all cases in the page, the opening paragraph is short enough that the image can be placed after the opening paragraph. Please make that change for all images, and adjust the paragraph text to refer to the "image below" rather than "above.

I'll have a few final copyedits to share after that. Thanks!

@chalin ,

I appreciate the thorough review you've given to the document. In my case, I believe that placing the diagram first offers some distinct advantages:

Context: Providing the diagram upfront supplies readers with a visual context before delving into the explanation, simplifying their comprehension of the content.

Engagement: Introducing a visual aid at the beginning captures readers' attention and piques their curiosity, increasing the likelihood that they'll engage with the subsequent text.

I hope you can understand my perspective on this. If it's not a strict requirement, I'd prefer to maintain the current format. While I recognize that there's no one-size-fits-all rule, my choice to place the diagrams first was deliberate. By investing in high-quality visuals, I aim to ensure that the related text is not overlooked and that the reader benefits from the full context.

Would you consider being open to this approach, as it may lead to a more engaging and effective document for our readers? I truly believe that this layout has the potential to enhance their overall experience.

Looking forward to your thoughts.
PS: Not limited but my favourite mentor on the subject 👉🏾 https://youtu.be/VwocXCyOieM

[chalin]

Thanks for the explanation @devrimdemiroz.

I agree with the advantages you are stating (engaging visuals etc) and IMHO, these advantages aren't lost by placing the images 2-3 lines further down the page.

One of the hats that I wear for this project is that of technical writer. In that role, I do my best to assure some form of consistency in the website content presentation, reasonably enforcing "best / recommended practices" that I've accumulated over the years. (Like trying to avoid "widowed or orphaned" headings.)

I'll let @svrnm @cartermp and @austinlparker share their thoughts, but for sizeable open-source projects like OTel, consistency in style and formatting across content (blog post are given a bit more flexibility), trumps personal style/preference, otherwise we'd end up with a degraded reader experience due to the use of disparate styles. Consistency is also why, for example, we've enforced the use of Prettier to format file content.

The only other solution I see, to keep the images first, would be to place the image in a figure before the heading, and then refer to the figure by number or title.

Cheers!

[austinlparker]

I'd agree with @chalin -- we should strive for consistency.

[linux-foundation-easycla]

• ✅ login: devrimdemiroz / name: Devrim Demiroz (6f126b5, 05840ac, 0a4dd2d, 264aae9, 055ec55, 8168769, 572cc07, 1b5fd33, bb8173d, ce595ed, 91619ec, e6a3b18, 1d5a737, 645a9c4, 36baab0, b7cedc1, 34b98d8, e11364e, 3a3d031, 0282208, 2e744ef, f24b274, cff3000, 204cae9, 05698f2, 1d5f904, 9da25b5, 0f9c60d, 7001c95, aae2cea, f6a68ce, 09b657c, ce1c92c, 809a0ab, 1018c5a, f3d9ed5, c7a72d1, 6762bf6, deb7c3a)
• ✅ login: chalin / name: Patrice Chalin (18614a1, c43058f)
• ❌ The commit (121862d). This user is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

[devrimdemiroz]

The Individual CLA option is not enabled for this project. Please to your administrator to enable the Individual CLA option for this project.

I wonder why individual CLA is not enabled 🧐 I do not recall such an error on opentelemetry-demo repository.

[austinlparker]

That's odd, it should be. I think there's a way to kick the EasyCLA check... @chalin ?

[chalin]

Hi @devrimdemiroz. Thanks for making the updates.

The CLA issue seems to be because you submitted the last commit from a different user ID, for which you haven't signed the CLA:

$ git log -2  
commit 121862d7923790ade0d3628a7d24cf43a229f31f (HEAD -> feature/otecol-data-flow-dashboard-PR797)
Author: Devrim <[email protected]>
Date:   Fri Mar 31 09:21:34 2023 +0200

    high above towards down below

commit c7a72d123618b886052ca43ab330f8581ffb04ae
Merge: f3d9ed5b 146de686
Author: Devrim Demiroz <[email protected]>
Date:   Fri Mar 31 09:09:57 2023 +0200

    Merge branch 'open-telemetry:main' into feature/otecol-data-flow-dashboard-PR797

I see two way's forward. Sign the CLA for the ID used in 121862d, or retract the commit and resubmit it using the same ID you did all the other work with.

[chalin]

I do not recall such an error on opentelemetry-demo repository.

I just checked the demo repo, and there you never used your ...+devrimdemiroz@users... ID.

[devrimdemiroz]

In any case, I made a new dummy commit that should resolve the issue. But still, EasyCLA check is in Required state.

[cartermp]

@devrimdemiroz I don't think the dummy commit helps since EasyCLA appears to check every commit. You could probably fixup that commit with an interactive rebase or re-open a new PR with the current document state (and none of the preceding commits). Either way, this is good enough for me to merge in when the license check is dealt with. I'd prefer not to use admin privilege to bypass that.

[devrimdemiroz]

@cartermp , I concluded same. I will go for opening a new PR and close this one. Thanks!

[devrimdemiroz]

Opened a new PR to bypass EasyCLA #2569
Closing this one...