Thanks for addressing my comments. I don't have anything else to add, but I can't sign off either.

Thanks for addressing my comments. I don't have anything else to add, but I can't sign off either.

No problem. Thanks a lot for the comments, they were very helpful.

The certificates have been regenerated through another PR. Once that gets merged, the tests should pass on this PR.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.01%. Comparing base (99dde77) to head (9144c04).
Report is 1 commits behind head on main.

@@           Coverage Diff           @@
##             main    #5743   +/-   ##
=======================================
  Coverage   95.01%   95.01%           
=======================================
  Files         303      303           
  Lines        7946     7946           
  Branches     1607     1607           
=======================================
  Hits         7550     7550           
  Misses        396      396           

It looks like the behavior here is not configurable. Given that the spec is in development, I think we should allow for users to disable or modify this in some way if they don't want it. I suggest we provide a configuration like redactedQueryParams and have the default value be the list of keys here. This would allow the user to override the list with one of their own if they want to add or remove redacted fields.

Makes sense. I will add this configuration.

@dyladan I have a question related to this configuration -
Do we only want to give the user capability to add more query strings if needed, along with the default strings provided in the specification. Or should we also provide the capability to -

• Remove some of the default query strings
• Overwrite the default values completely

@pichlermarc, @dyladan for now I have implemented the logic, which allows users to add values but the default set provided in the specification will always be redacted. Please take a look at the changes and advise if the logic needs to be revised or if we should proceed with this approach.

Please try to avoid changing history with rebase and force push. It prevents previous comments from being properly linked to the code in the PR

@dyladan Apologies, I completely forgot about the comment for not rebasing. For this change, I did a rebase.

I think the behavior of redactedQueryParams is potentially confusing

• redactedQueryParams == undefined -> use default list (good)
• redactedQueryParams == ['my-attr'] -> use list passed by user (good)
• redactedQueryParams == [] -> use default list (confusing) - in this case I'd expect no params to be redacted, since the list of redacted params is empty

I think I would expect the empty list to have the effect of disabling redaction if I was the user. What do you think?

Yes, sounds good to me. I have added support for this configuration. Could you please take another look at it when you get a chance.

Thank you for your contribution @rads-1996! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.