-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Align OTEL http attributes with latest standard #121
Conversation
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
I'd encourage you to check in the retry handler which most likely has some of those headers as well.
I've searched the whole project for attributes listed in #121 and replaced them all. Retry handler has retry_count attribute mentioned in #119, but I wanted to create another PR for it, because I'm not sure about implementation of retry_delay attribute. "int duration in seconds before the request was retried" Does it mean number of seconds between every retry or total sum for all retries? |
Thank you for the additional information. The attributes in the retry handler are both outdated AND different from the fullfill/reject implementations...
The areas I could find
And to answer your question: retry delay I think it the delay of each retry (not the total delay). Let us know if you have any additional comments or questions. |
also the status code attribute is missing in this handler. |
I added second commit to update resend_count and resend_delay attributes and add status_code attribute in RetryHandler class. If everything is ok, it closes #119 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for making the changes!
Can you add a new changelog entry (patch bump, today's date, changed) please?
As well as align the version constant
This will ensure a swift release.
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for making the changes!
Closes #120