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

Unable to render rich display when generated telemetry contains mermaid keywords #76

Open
chancez opened this issue Jul 23, 2024 · 0 comments · May be fixed by #77
Open

Unable to render rich display when generated telemetry contains mermaid keywords #76

chancez opened this issue Jul 23, 2024 · 0 comments · May be fixed by #77

Comments

@chancez
Copy link

chancez commented Jul 23, 2024 •
edited
Loading

In Cilium we're using this action and hitting an issue with the rendered mermaid charts: cilium/cilium#32241 (comment)

After some investigation, I think we are hitting this bug mermaid-js/mermaid#2495.

Basically, if the telemetry action generates a chart with words like "call" in the node text, it breaks rendering due to the mermaid bug mentioned above.

Here's a minimal example that fails

gantt
	title Some title
	dateFormat x
	axisFormat %H:%M:%S

	Call blah blah blah : 1721689506000, 1721689507000
Loading

And it works if you remove call:

gantt
	title Some title
	dateFormat x
	axisFormat %H:%M:%S

	blah blah blah : 1721689506000, 1721689507000
Loading

I've actually never used mermaid charts myself yet, so I'm not sure how easy it is to fix, but according to mermaid-js/mermaid#2495 (comment) you can wrap the nodes in quotes to fix the issue?

So eg:

gantt
	title Some title
	dateFormat x
	axisFormat %H:%M:%S

	"Call blah blah blah" : 1721689506000, 1721689507000
Loading

Seems to work 🤷 . Maybe the action can be updated to quote everything to fix the issue?
chancez added a commit to chancez/workflow-telemetry-action that referenced this issue Jul 23, 2024
@chancez
Quote step names to work around mermaid-js/mermaid#2495
d41dfbc 
Fixes catchpoint#76

Signed-off-by: Chance Zibolski <[email protected]>
@chancez chancez linked a pull request Jul 23, 2024 that will close this issue
Open
chancez added a commit to chancez/workflow-telemetry-action that referenced this issue Jul 24, 2024
@chancez
Quote step names to work around mermaid-js/mermaid#2495
881045c 
Fixes catchpoint#76

Signed-off-by: Chance Zibolski <[email protected]>
chancez added a commit to cilium/cilium that referenced this issue Jul 30, 2024
@chancez
Fix workflow telemetry in ci-ipsec-upgrade
ebf18ce 
Basically, workflow telemetry uses step names for gantt charts which
uses Github's native support for mermaidjs for rendering the charts in
issues and PRs. However mermaidjs has an issue
(mermaid-js/mermaid#2495) when you use
mermaid reserved keywords in node names/text.

To fix rendering of the ipsec upgrade workflow's telemetry, avoid the
use of the "call" keyword.

This is a work around for catchpoint/workflow-telemetry-action#76.
If an when catchpoint/workflow-telemetry-action#77 is merged, we do not need to be concerned with workflow step names.

Fixes #32241

Signed-off-by: Chance Zibolski <[email protected]>
@chancez chancez mentioned this issue Jul 30, 2024
Merged
github-merge-queue bot pushed a commit to cilium/cilium that referenced this issue Jul 31, 2024
@chancez @julianwiedmann
Fix workflow telemetry in ci-ipsec-upgrade
9592c69 
Basically, workflow telemetry uses step names for gantt charts which
uses Github's native support for mermaidjs for rendering the charts in
issues and PRs. However mermaidjs has an issue
(mermaid-js/mermaid#2495) when you use
mermaid reserved keywords in node names/text.

To fix rendering of the ipsec upgrade workflow's telemetry, avoid the
use of the "call" keyword.

This is a work around for catchpoint/workflow-telemetry-action#76.
If an when catchpoint/workflow-telemetry-action#77 is merged, we do not need to be concerned with workflow step names.

Fixes #32241

Signed-off-by: Chance Zibolski <[email protected]>
nbusseneau pushed a commit to cilium/cilium that referenced this issue Aug 3, 2024
@chancez @nbusseneau
Fix workflow telemetry in ci-ipsec-upgrade
724c10c 
[ upstream commit 9592c69 ]

Basically, workflow telemetry uses step names for gantt charts which
uses Github's native support for mermaidjs for rendering the charts in
issues and PRs. However mermaidjs has an issue
(mermaid-js/mermaid#2495) when you use
mermaid reserved keywords in node names/text.

To fix rendering of the ipsec upgrade workflow's telemetry, avoid the
use of the "call" keyword.

This is a work around for catchpoint/workflow-telemetry-action#76.
If an when catchpoint/workflow-telemetry-action#77 is merged, we do not need to be concerned with workflow step names.

Fixes #32241

Signed-off-by: Chance Zibolski <[email protected]>
Signed-off-by: Nicolas Busseneau <[email protected]>
nbusseneau pushed a commit to cilium/cilium that referenced this issue Aug 3, 2024
@chancez @nbusseneau
Fix workflow telemetry in ci-ipsec-upgrade
c1ad81f 
[ upstream commit 9592c69 ]

Basically, workflow telemetry uses step names for gantt charts which
uses Github's native support for mermaidjs for rendering the charts in
issues and PRs. However mermaidjs has an issue
(mermaid-js/mermaid#2495) when you use
mermaid reserved keywords in node names/text.

To fix rendering of the ipsec upgrade workflow's telemetry, avoid the
use of the "call" keyword.

This is a work around for catchpoint/workflow-telemetry-action#76.
If an when catchpoint/workflow-telemetry-action#77 is merged, we do not need to be concerned with workflow step names.

Fixes #32241

Signed-off-by: Chance Zibolski <[email protected]>
Signed-off-by: Nicolas Busseneau <[email protected]>
gandro pushed a commit to cilium/cilium that referenced this issue Aug 6, 2024
@chancez @gandro
Fix workflow telemetry in ci-ipsec-upgrade
dbb0a35 
[ upstream commit 9592c69 ]

Basically, workflow telemetry uses step names for gantt charts which
uses Github's native support for mermaidjs for rendering the charts in
issues and PRs. However mermaidjs has an issue
(mermaid-js/mermaid#2495) when you use
mermaid reserved keywords in node names/text.

To fix rendering of the ipsec upgrade workflow's telemetry, avoid the
use of the "call" keyword.

This is a work around for catchpoint/workflow-telemetry-action#76.
If an when catchpoint/workflow-telemetry-action#77 is merged, we do not need to be concerned with workflow step names.

Fixes #32241

Signed-off-by: Chance Zibolski <[email protected]>
Signed-off-by: Nicolas Busseneau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Quote step names to work around mermaid-js/mermaid#2495 chancez/workflow-telemetry-action
1 participant
@chancez