open-telemetry / build-tools

Building tools provided by OpenTelemetry
https://opentelemetry.io
Apache License 2.0
37 stars 54 forks source link

Generator needs better support for internal and spec links. #20

Open Oberon00 opened 3 years ago

Oberon00 commented 3 years ago

See https://github.com/open-telemetry/opentelemetry-specification/pull/1192

The markdown generator would need to be passed an URL prefix that is treated as root which is relatively linkable. Semantic conventions would contain absolute links (i.e. revert that part of https://github.com/open-telemetry/opentelemetry-specification/pull/1192).

Other generators should support replacing an URL prefix e.g. https://github.com/open-telemetry/opentelemetry-specification/tree/master/ to https://github.com/open-telemetry/opentelemetry-specification/tree/v0.6/.

jkwatson commented 3 years ago

In addition, it would be very helpful if there were a way to change links from being markdown to being standard html links.

Oberon00 commented 3 years ago

@jkwatson I did not know myself, but we already merged support for that: https://github.com/open-telemetry/build-tools/blob/main/semantic-conventions/src/opentelemetry/semconv/templating/code.py#L28

jkwatson commented 3 years ago

@jkwatson I did not know myself, but we already merged support for that: https://github.com/open-telemetry/build-tools/blob/main/semantic-conventions/src/opentelemetry/semconv/templating/code.py#L28

I looked through the code a bunch last week, but isn't that method for "rendering markdown", not the jinja templates?

Oberon00 commented 3 years ago

That function is made available to the code generator template here: https://github.com/open-telemetry/build-tools/blob/main/semantic-conventions/src/opentelemetry/semconv/templating/code.py#L204

So you should be able to do {{attr.brief | render_markdown()}} in the Jinja template.

jkwatson commented 3 years ago

That function is made available to the code generator template here: https://github.com/open-telemetry/build-tools/blob/main/semantic-conventions/src/opentelemetry/semconv/templating/code.py#L204

So you should be able to do {{attr.brief | render_markdown()}} in the Jinja template.

I'll give it a whirl and see how it goes. I never would have found that particular bit of code, or had any idea how to use it. Thanks for the pointer!

Oberon00 commented 3 years ago

Yeah, the docs are lacking 😔

jkwatson commented 3 years ago

Unfortunately, render_markdown doesn't render it in a javadoc-friendly manner. it escapes all the html tags, even ones that are good for javadoc (<p> and <code> for instance). Is there a magic way to stop that from happening?

jkwatson commented 3 years ago

It looks like you might be able to pass in a function for formatting the particular bits...not sure how to call that from a jinja template, but I'm still poking at it.

jkwatson commented 3 years ago

I got something sort-of working: https://github.com/open-telemetry/opentelemetry-java/pull/3394

jsuereth commented 6 months ago

We discussed some of this in a recent small-group on Semantic Convention Tooling. I think we'll need to tear this feature request into pieces, but a few points:

  1. Semconv URLs in markdown must support the website (opentelemtery.io) rewrite rules. Whatever we generate in build-tools should be compatible in someway there.
  2. We want to tie specific versions of semconv to specific versions of the specification, so making sure links to the specification all come from the same version is important. As such, a general command line argument is best.
  3. Today - the "is_local" flag that infers if the markdown + yaml match the same directory structure is a bit too "tied together". Since we've decided to move towards an attribute registry, we think:
    • There should be a consistent "shape" to the registry
    • The registry should be built with templates vs. manually creating Markdown and embedding templates
    • Markdown snippet generation we use in docs for semconv should simple know the rules for how to link to the attribtue registry, but we won't use snippet generation to PRODUCE the attribute registry going forward.

Given this - I think we should split this feature request apart between "internal" and "spec" links.

Specifically: