opentelemetry-specification icon indicating copy to clipboard operation
opentelemetry-specification copied to clipboard

Clarify the HTTP client `http.retry_count` spec

Open mateuszrzeszutek opened this issue 2 years ago • 8 comments

Fixes #2737

Changes

Clarifies when the http.retry_count attribute should be added to the HTTP client span (every time the HTTP request gets re-sent, no matter what the cause is).

I also added a grouping "Examples" header for all examples, similar to other (messaging, DB) convention docs.

mateuszrzeszutek avatar Aug 19 '22 10:08 mateuszrzeszutek

I just have a question on naming. The attribute is retry but in the text we use the word re-send/sent. Not sure if it's a problem but I wonder if we shouldn't be consistent?

I'm in favor of this idea; initially I didn't want to change too much of the spec, just clarify things, but if more people than just me think it's a valid point I think we could rename it to resend_count. (I also mentioned this in https://github.com/open-telemetry/opentelemetry-specification/pull/2743#discussion_r951400886)

mateuszrzeszutek avatar Aug 22 '22 13:08 mateuszrzeszutek

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions[bot] avatar Aug 30 '22 04:08 github-actions[bot]

@open-telemetry/specs-approvers @open-telemetry/technical-committee please take a look at this PR

cc @lmolkova (sorry, I completely forgot to tag you before 🙈)

mateuszrzeszutek avatar Aug 30 '22 12:08 mateuszrzeszutek

Sorry, forgot to hit send in one comment: You probably need to add an entry to the changelog for this as well, otherwise looks good to me.

joaopgrassi avatar Sep 07 '22 15:09 joaopgrassi

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions[bot] avatar Sep 17 '22 03:09 github-actions[bot]

/unstale

trask avatar Sep 17 '22 19:09 trask

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions[bot] avatar Oct 13 '22 04:10 github-actions[bot]

Ping @open-telemetry/technical-committee : is this PR ready to be merged? Is there anything else that I can help with?

mateuszrzeszutek avatar Oct 13 '22 09:10 mateuszrzeszutek