howardyoo commented on PR #43340:
URL: https://github.com/apache/airflow/pull/43340#issuecomment-2452458443
I can def check whether the name validation works on python otel sdk side,
as well as on collector side.Will update sometime this weekendHowardSent from
my iPhoneOn Nov 1, 2024, at 1:31 PM, D. Ferruzzi ***@***.***> wrote:
@ferruzzi commented on this pull request.
In airflow/metrics/otel_logger.py:
> @@ -300,6 +305,15 @@ def timer(
"""Timer context manager returns the duration and can be
cancelled."""
return _OtelTimer(self, stat, tags)
+ def get_name(self, metric_name: str, tags: Attributes | None = None) ->
str:
I agree if OTel is doing that now and it's working as expected, then we can
let it. I also think that's getting out of scope for this PR though. I think
we can implement this "get name" and worry about testing and possibly pruning
out the name validation if they are doing it on their end.
I also agree with Howard that we can and should keep the OTel names short
with tags and continue to emit the long concatenated name for StatsD, which
means this get_name implementation is still needed/useful either way.
How do you feel about merging this, and Arshia can experiment with removing
the name validation we have implemented and make sure the otel validation is
actually working as advertised? I'm the one who initially implemented this in
our codebase and I wouldn't have done all that name validation code if their
validation was actually working. I remember getting frustrated trying to
figure out the "undocumented" length limit at the time. If they've fixed it
now then by all means let's clean our code up!
—Reply to this email directly, view it on GitHub, or unsubscribe.You are
receiving this because you were mentioned.Message ID: ***@***.***>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
