[Instrumentation.Owin] Add IConfiguration support #1973
Conversation
github-actions
bot
added
the
comp:instrumentation.owin
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1973 +/- ##
===========================================
+ Coverage 73.91% 85.79% +11.88%
===========================================
Files 267 8 -259
Lines 9615 169 -9446
===========================================
- Hits 7107 145 -6962
+ Misses 2508 24 -2484
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -17,18 +18,20 @@ public class OwinInstrumentationOptions | |||
| /// Initializes a new instance of the <see cref="OwinInstrumentationOptions"/> class. | |||
| /// </summary> | |||
| public OwinInstrumentationOptions() | |||
| : this(new ConfigurationBuilder().AddEnvironmentVariables().Build()) | |||
There was a problem hiding this comment.
Wouldn't it be the best option to take IConfiguration from the user instead of building it with environment variables ourselves? Is the intention here to avoid a public API change?
There was a problem hiding this comment.
Ya this is just about backwards compatibility and not breaking the public API. Nothing should be using this ctor but there is a chance some user is calling it. Perhaps in a test. But this is a prerelease package so we could break it and remove this ctor leaving just have the internal one. LMK what you think.
There was a problem hiding this comment.
I'm fine with this approach.
Kielek
changed the title
Kielek
changed the title
Relates to #1960
Changes
OTEL_DOTNET_EXPERIMENTAL_OWIN_DISABLE_URL_QUERY_REDACTIONusingIConfigurationMerge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changes