search

open-telemetry / weaver

OTel Weaver lets you easily develop, validate, document, and deploy semantic conventions
Apache License 2.0
58 stars 23 forks source link

Custom Capitalizations #415

Open MrAlias opened 1 month ago

MrAlias commented 1 month ago

The Go team currently uses the following capitalization for known acronyms and initialisms:

The current build tooling ensures these capitalizations are used in any naming generated. How can we keep this behavior when using weaver?

lquerel commented 1 month ago

There is a section in the Weaver config just for that and a corresponding Jinja filter :-)

Exemple:

{{ attr.brief | acronym }}
MrAlias commented 1 month ago

Ah, thanks. I missed that. I'll give it a try.

MrAlias commented 1 month ago

It looks like that is already being used:

https://github.com/open-telemetry/opentelemetry-go/pull/5793/files#diff-c27c5af791853e678c1931568a666a2a468a25035fd7412fb8907f67ccad89d1R52-R146

https://github.com/open-telemetry/opentelemetry-go/pull/5793/files#diff-f65227d0a0712ab4e3e66dbedcbac72947e6e310b0d4317c1a5d5f26fee96009R2

But it is still generating invalid names. For example:

lquerel commented 1 month ago

That’s strange. The latest links you added to your last comment aren’t working for me. Could you point me to the file and the line where you observed the issue for AWS? I’d like to determine if it’s a flaw in the acronym filter or a usage problem.

MrAlias commented 1 month ago

@lquerel thanks for the response.

Those files are from this PR: https://github.com/open-telemetry/opentelemetry-go/pull/5793

I'm working in that branch and able to reproduce the results generated there. I have updated to use v0.10.0 as well and still not able to remedy the naming.

MrAlias commented 1 month ago

FWIW, I have also tried moving the acronym function earlier in the processing:

{%- macro to_go_name(fqn) -%}
{{ fqn | acronym | title_case | replace(" ", "") }}
{%- endmacro -%}

Still no change.

MrAlias commented 1 month ago

Removing the title_case function resulted in things like the following to be produced:

AWS.DynamoDB.consistent_readKey

Which seems to show that the acronym function is working but not interacting well with the title_case.

Is there a way to have these work together?

lquerel commented 1 month ago

Could you try the following macro

{%- macro to_go_name(fqn) -%}
{{ fqn | title_case | acronym | replace(" ", "") }}
{%- endmacro -%}

Applying the title_case filter after the acronym filter destroys the effect of the acronym filter. If that doesn't work please copy/paste the result of fqn | title_case without anything else. Thanks

MrAlias commented 1 month ago 
{%- macro to_go_name(fqn) -%}
{{ fqn | title_case | acronym | replace(" ", "") }}
{%- endmacro -%}

AwsDynamodbConsistentReadKey

{%- macro to_go_name(fqn) -%}
{{ fqn | title_case }}
{%- endmacro -%}

AwsDynamodbConsistentReadKey

lquerel commented 1 month ago

Okay, I see. The acronym filter works at the word boundary level, while the title_case filter transforms, in this case, a dot notation (e.g., aws.dynamodb.consistent.readKey) into a single word (e.g., AwsDynamodbConsistentReadKey). I need to consider the best way to fix this issue. I’ll include a fix in the next release.