refactor!: prometheus attributes format to otel attributes

[varshith257]

/claim #4057
Closes #4057

[varshith257]

@sergiuszkierat I just worked this a few days ago and haven't pushed here :)

[varshith257]

Just need tweaks here to this PR. Once it is done I will tag you here

[sergiuszkierat]

@sergiuszkierat I just worked this a few days ago and haven't pushed here :)

@varshith257 Thanks for letting me know. Better late than never 😅 I can freely switch to another task

[varshith257]

@adamw It's ready for your review

[varshith257]

Will definitely look into it

…

On Thu, 31 Oct, 2024, 2:41 pm Sergiusz Kierat, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In metrics/opentelemetry-metrics/src/main/scala/sttp/tapir/server/metrics/opentelemetry/OpenTelemetryMetrics.scala <#4122 (comment)>: > @@ -80,7 +80,7 @@ object OpenTelemetryMetrics { def requestActive[F[_]](meter: Meter, labels: MetricLabels): Metric[F, LongUpDownCounter] = Metric[F, LongUpDownCounter]( meter - .upDownCounterBuilder("request_active") + .upDownCounterBuilder("http.server.active_requests") are all metrics that we should report, included here, or is there anything missing? I guess that having those optional metrics (http.server.request.body.size <https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#metric-httpserverrequestbodysize> , http.server.response.body.size <https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#metric-httpserverresponsebodysize>) would be useful 😁 — Reply to this email directly, view it on GitHub <#4122 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A4BF3HCGZPMQ2TVJIAY4MP3Z6HX4JAVCNFSM6AAAAABQ2Q6GUGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMBXGQZDMMJXG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[varshith257]

@adamw Done!

[adamw]

I think if the response is a Right we shouldn't add any attributes

[varshith257]

Yes, it will helps.Will make changes for it

[varshith257]

@adamw The scala compiler is hungry to handle Right case?
WDYT the better way what you are thinking of?

[varshith257]

@adamw

[adamw]

ah, well I guess this would have to return an Option[String], not a String, and then you would report only the defined labels

[varshith257]

@adamw It's getting a bit tricky making "error.type" to Option[String]. In that case, if we make forResponse then h http.response.status_code could go None for bad req?

[varshith257]

I made this comment 3 days before but don't know why not posted here lol

[adamw]

yes, if an attribute is None we don't include it, if it's a Some we do

[varshith257]

@adamw Can you approve workflows? I have no access to my desktop for now and I have made this commit through mobile :)

[adamw]

I'm closing this due to inactivity - please reopen if you'd like to work on the issue.

[varshith257]

@adamw I have occupied with my university exams so I haven't made time to look close again. I just bit lost track of it. I think the only review work left is to make optional attributes. I have tried in my local but doesn't work what we outlined it expecting for Right.

Do you have any suggestions on it?

[adamw]

@varshith257 well you have to change the code so that it works with optionals. I won't write the code for you, sorry :)

[varshith257]

@adamw I mean do you have any suggestions to make it to get work done quickly? Or else I will take a look again and fix it this weekend, my exams will be completed

[adamw]

@varshith257 no, you have to make this work with optionals, there's no cutting corners. I'll only accept solutions which fully meet the initial criteria and are well-written.