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

Raise warnings when special tags are misspelled, or missing spaces #501

Open
ChrsMark opened this issue Dec 4, 2024 · 5 comments
Open
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@ChrsMark
Copy link
Member

ChrsMark commented Dec 4, 2024

Describe the bug

A small "typo" like <!-- semconv metric.k8s.hpa.min_pods--> (missing space before -->) makes the tooling producing no markdown tables without raising a warning for this.

Raising a warning would definitely help on troubleshooting cause usually these kind of typos are hard to spot :).

To Reproduce

Use a metric tag like <!-- semconv metric.k8s.hpa.min_pods--> and try to generate the markdowns (make fix).

Expected behavior
A clear and concise description of what you expected to happen.

At least a warning should let users know what wrong with that tag.

Additional context
Add any other context about the problem here.

ref: https://cloud-native.slack.com/archives/C041APFBYQP/p1733311696182559

@jsuereth jsuereth added bug Something isn't working good first issue Good for newcomers labels Dec 4, 2024
@jsuereth
Copy link
Contributor

jsuereth commented Dec 4, 2024

I looked into this, the issue is my parser isn't accurately pulling HTML comments and then parsing the content within it.

link to code

Specifically, it's requiring space to parse the id. That is, we're parsing:

  • <!--: HTML header
  • semconv metric.k8s.hpa.min_pods-->: Identifier
  • ERROR!

I think this could be fixed by created a generic HTML comment parser that captures the content between <-- and -->, and then feeding this to the semconv controls parser.

I'll try to get to this after my latest Go bug fixes unless someone else has time to address it.

@yluobb
Copy link

yluobb commented Dec 6, 2024

Hello, I’m new to contributing to open source projects and I thought this would be a beginner-friendly issue, can I attempt this?

@jsuereth
Copy link
Contributor

jsuereth commented Dec 6, 2024

@yluobb Feel free! I did create this PR: #512 which doesn't solve the problem but should provide a good help in solving the problem.

@yluobb
Copy link

yluobb commented Dec 6, 2024

@jsuereth It looks like the changes in your PR works fine with the current tests. Could you provide a specific example or input where the current solution isn't working as expected? This will help me better understand the issue.

@yluobb
Copy link

yluobb commented Dec 8, 2024

@jsuereth I added the mechanism to raise warning when there are missing spaces here, please feel free to take a look, and please let me know what other fix I can help with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants