Obfuscate ids with commas and obfuscate WCF requests#3026
Conversation
|
Note: Still working on updating the integration/end-to-end tests for |
bouwkast
changed the title
bouwkast
added
area:tracer
Thinking on this more, I think this PR and the ASM obfuscation can be handled separately. The obfuscation we want to achieve in this PR affects the resourceName whereas the ASM changes would affect the
That's a good question and yeah it definitely is 😬 In this case, we have only seen one complaint about the Sometimes we will add an opt-in feature flag and then over time make it opt-out during a new major version release. However, this seems like a bug fix so I'd advocate for making this the default behavior and, if needed, we could create an opt-out flag |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
30f4423 to
eb39869
Compare
I've updated the namespace that was being used in the sample to include |
| } | ||
|
|
||
| continue; | ||
| case ',': |
There was a problem hiding this comment.
we can't have hexa with commas right? in which case we should return false if we have a letter and a segment > 16?
There was a problem hiding this comment.
I'm not sure if I follow
Are you asking if hex segments: > 16 characters, that contains at least one letter and one comma that we shouldn't obfuscate it?
There was a problem hiding this comment.
I'm saying that I've never seen a 123af,12a for an hexa value. So if I'm right, then in the case where we find a letter and a comma, then it's not an id. So we should not obfuscate it.
Also, what about points? Should we add them here?
(Maybe that's a good time to go and look at what others tracers have done to see if we are close as well.)
There was a problem hiding this comment.
Maybe that's a good time to go and look at what others tracers have done to see if we are close as well
I need to do that as part of the RFC on obfuscating outgoing Http requests too, so would be good to document them all!
There was a problem hiding this comment.
I'm saying that I've never seen a 123af,12a for an hexa value. So if I'm right, then in the case where we find a letter and a comma, then it's not an id. So we should not obfuscate it.
Okay that makes sense.
So "123af,12a" currently doesn't get obfuscated because the segmentLength is less than 16, but you'd also expect that to still return false if the segmentLength is greater than 16 because it is no longer an "id".
Something like "123afafafafaf,12a" does get obfuscated with ? currently, but ideally it wouldn't. Should be able to add a check for that
I will go through the other tracers and see if I can see what they do and will document along the way :)
There was a problem hiding this comment.
I need to do that as part of the RFC on obfuscating outgoing Http requests too, so would be good to document them all!
:itsatrap: :p
Something like "123afafafafaf,12a" does get obfuscated with ? currently, but ideally it wouldn't. Should be able to add a check for that
I assume yes.
I will go through the other tracers and see if I can see what they do and will document along the way :)
Thanks a lot. Don't hesitate to ask for pointers in #apm-integrations. Also, you can use codesearch, but that doesn't prevent you to ask to others if the search isn't obvious
|
|
||
| span.DecorateWebServerSpan( | ||
| resourceName: action ?? requestHeadersTo?.LocalPath, | ||
| resourceName: string.IsNullOrEmpty(action) ? UriHelpers.GetCleanUriPath(requestHeadersTo?.LocalPath) : action, |
There was a problem hiding this comment.
Maybe add a feature flag (enabled by default) and do the obfuscation only when it is set. That will give the users a way to rollback if needed.
There was a problem hiding this comment.
That sounds like a good idea, I was wondering whether or not if this would be too much of a behavioral breaking change.
Follow up question for this, how exactly are feature flags implemented? Is that in ConfigurationKeys and the FeatureFlags class or elsewhere?
There was a problem hiding this comment.
Follow up question for this, how exactly are feature flags implemented? Is that in ConfigurationKeys and the FeatureFlags class or elsewhere?
Yes
There was a problem hiding this comment.
Is that in ConfigurationKeys and the FeatureFlags class or elsewhere
That's the one, and then you need to add a setting to TracerSettings and ImmutableTracerSettings to flow it through. I would be inclined to call it something like DD_TRACE_WCF_RESOURCE_OBFUSCATION_ENABLED, default it true, so they can disable it if it causes issues.
Please also record the value of that flag in the tracer start up log + telemetry too, so we can track when/if people need to disable it 🙂
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| /// <summary> | ||
| /// Note: after updating this interface you'll need to re-generate the proxy class <see cref="CalculatorClient"/>. | ||
| /// That can be done in a developer VS prompt (when the WCF server is running): | ||
| /// <code>svcutil.exe /language:cs /out:generatedProxy.cs /config:app.config http://localhost:8585/WcfSample/</code> | ||
| /// This makes a new file <c>generatedProxy.cs</c> and the contents of this can be swapped out with <see cref="CalculatorClient"/>. | ||
| /// </summary> |
There was a problem hiding this comment.
Nice, thanks for adding this 👍
There was a problem hiding this comment.
Thank you for adding this, if we ever need to update this again this will be extremely helpful 🙏🏼
| SpanId: Id_2, | ||
| Name: wcf.request, | ||
| Resource: WcfSample/ICalculator/ServerSyncAdd, | ||
| Resource: /WcfSample/?/CalculatorService, |
There was a problem hiding this comment.
Just so I understand, this one has the URL rather than a useful name because it's the one with Action="" right? I wonder if we should strip the leading / from the URL, just so it looks similar to the ones with an action? Or maybe it's better that it's different, so it can't be confused for the others? 🤔 Open to discussion!
There was a problem hiding this comment.
Correct. For the Action="" it falls back to using the LocalPath for the resource name (also I couldn't figure out how to get Action to be null without the server throwing).
Making them look similar would make sense to me - since we do lose the "useful" name maybe it's better to not though? If anyone asks why they aren't getting the Action in the resource name and they have that leading / we know that the Action was actually empty or null and they were falling back to the URL form? :)
I didn't think anything of that leading / so I'd be fine either stripping it out or leaving it in
There was a problem hiding this comment.
I would prefer keeping the leading / to help differentiate between an action and a path. For aspnet.request spans we do something similar for the resource names, where we give it a pattern <HTTP_METHOD> <PATH> where path has a leading /, so this would be consistent
There was a problem hiding this comment.
nit: One thing you could do to make the diff in the snapshots a little clearer is add the new test case to the end so the bulk of the changes are in the last span in the file, but definitely not a requirement Scratch that, that would just move the problem down a few lines. Nevermind!
|
|
||
| span.DecorateWebServerSpan( | ||
| resourceName: action ?? requestHeadersTo?.LocalPath, | ||
| resourceName: string.IsNullOrEmpty(action) ? UriHelpers.GetCleanUriPath(requestHeadersTo?.LocalPath) : action, |
There was a problem hiding this comment.
Is that in ConfigurationKeys and the FeatureFlags class or elsewhere
That's the one, and then you need to add a setting to TracerSettings and ImmutableTracerSettings to flow it through. I would be inclined to call it something like DD_TRACE_WCF_RESOURCE_OBFUSCATION_ENABLED, default it true, so they can disable it if it causes issues.
Please also record the value of that flag in the tracer start up log + telemetry too, so we can track when/if people need to disable it 🙂
zacharycmontoya
left a comment
zacharycmontoya
left a comment
There was a problem hiding this comment.
Looks good, this should be ready to go once the feature flag is added
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
zacharycmontoya
left a comment
zacharycmontoya
left a comment
There was a problem hiding this comment.
LGTM, nice work!
ac3102c to
3dfd78d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This would replace "123,123" with "?". Previously, nothing would be replaced.
This reverts commit 8ca8402.
If an OperationContract attribute is defined with an empty string ("")
Action we'll use that empty string as the `resourceName`. So check for
that and swap to use the LocalPath instead.
Previously we were using the non-obfuscated LocalPath of the URI, instead we can use the cleaned and obfuscated AbsolutePath of the URI.
Using and cleaning the whole URI instead of just the LocalPath was causing the actual content in the `resourceName` to be different than what it was previously.
Now when the Action is an empty string it'll fall back to the LocalPath and should properly obfuscate away the "123,123" with a "?" in the resourceName.
0721ebc to
fce893b
Compare
Snapshots difference summaryThe following differences have been observed in snapshots. So diff is simplistic, so please check some files anyway while we improve it. 4 occurrences of : + {
+ TraceId: Id_1,
+ SpanId: Id_2,
+ Name: wcf.request,
+ Resource: /WcfSample/123,123/CalculatorService,
+ Service: Samples.Wcf,
+ Type: web,
+ ParentId: Id_3,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.method: POST,
+ http.request.headers.host: localhost:00000,
+ http.url: http://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_4,
+ SpanId: Id_5,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerSyncAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ ParentId: Id_6,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.method: POST,
+ http.request.headers.host: localhost:00000,
+ http.url: http://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_7,
+ SpanId: Id_8,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerSyncAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ ParentId: Id_9,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.method: POST,
+ http.request.headers.host: localhost:00000,
+ http.url: http://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_10,
+ SpanId: Id_11,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerSyncAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ ParentId: Id_12,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.method: POST,
+ http.request.headers.host: localhost:00000,
+ http.url: http://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_13,
+ SpanId: Id_14,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerTaskAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ ParentId: Id_15,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.method: POST,
+ http.request.headers.host: localhost:00000,
+ http.url: http://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_16,
+ SpanId: Id_17,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerTaskAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ ParentId: Id_18,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.method: POST,
+ http.request.headers.host: localhost:00000,
+ http.url: http://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_19,
+ SpanId: Id_20,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerTaskAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ ParentId: Id_21,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.method: POST,
+ http.request.headers.host: localhost:00000,
+ http.url: http://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ }
4 occurrences of : - Resource: WcfSample/ICalculator/ServerSyncAdd,
+ Resource: /WcfSample/?/CalculatorService,
20 occurrences of : - http.url: http://localhost:00000/WcfSample/CalculatorService,
+ http.url: http://localhost:00000/WcfSample/123,123/CalculatorService,
4 occurrences of : - Resource: WcfSample/ICalculator/ServerTaskAdd,
+ Resource: WcfSample/ICalculator/ServerSyncAdd,
4 occurrences of : - http.url: http://localhost:00000/WcfSample/CalculatorService,
+ http.url: http://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_19,
+ SpanId: Id_20,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerTaskAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ ParentId: Id_21,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.method: POST,
+ http.request.headers.host: localhost:00000,
+ http.url: http://localhost:00000/WcfSample/123,123/CalculatorService,
2 occurrences of : + {
+ TraceId: Id_1,
+ SpanId: Id_2,
+ Name: wcf.request,
+ Resource: /WcfSample/123,123/CalculatorService,
+ Service: Samples.Wcf,
+ Type: web,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.agent_psr: 1.0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_3,
+ SpanId: Id_4,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerSyncAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.agent_psr: 1.0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_5,
+ SpanId: Id_6,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerSyncAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.agent_psr: 1.0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_7,
+ SpanId: Id_8,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerSyncAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.agent_psr: 1.0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_9,
+ SpanId: Id_10,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerTaskAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.agent_psr: 1.0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_11,
+ SpanId: Id_12,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerTaskAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.agent_psr: 1.0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_13,
+ SpanId: Id_14,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerTaskAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.agent_psr: 1.0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ }
2 occurrences of : - Resource: WcfSample/ICalculator/ServerSyncAdd,
+ Resource: /WcfSample/?/CalculatorService,
[...]
- http.url: net.tcp://localhost:00000/WcfSample/CalculatorService,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
6 occurrences of : - http.url: net.tcp://localhost:00000/WcfSample/CalculatorService,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
2 occurrences of : - Resource: WcfSample/ICalculator/ServerTaskAdd,
+ Resource: WcfSample/ICalculator/ServerSyncAdd,
[...]
- http.url: net.tcp://localhost:00000/WcfSample/CalculatorService,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
2 occurrences of : - http.url: net.tcp://localhost:00000/WcfSample/CalculatorService,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
+ language: dotnet,
+ runtime-id: Guid_1,
+ span.kind: server,
+ version: 1.0.0,
+ _dd.p.dm: -0
+ },
+ Metrics: {
+ process_id: 0,
+ _dd.agent_psr: 1.0,
+ _dd.top_level: 1.0,
+ _dd.tracer_kr: 1.0,
+ _sampling_priority_v1: 1.0
+ }
+ },
+ {
+ TraceId: Id_13,
+ SpanId: Id_14,
+ Name: wcf.request,
+ Resource: WcfSample/ICalculator/ServerTaskAdd,
+ Service: Samples.Wcf,
+ Type: web,
+ Tags: {
+ component: Wcf,
+ env: integration_tests,
+ http.url: net.tcp://localhost:00000/WcfSample/123,123/CalculatorService,
|
Benchmarks Report 🐌Benchmarks for #3026 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AppSecBodyBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AspNetCoreBenchmark - Unknown 🤷 Same allocations ✔️Raw results
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️Raw results
|
Code Coverage Report 📊✔️ Merging #3026 into master will not change line coverage
View the full report for further details: Datadog.Trace Breakdown ✔️
The following classes have significant coverage changes.
View the full reports for further details: |
5 participants
Summary of changes
Obfuscate ids that contain
,(123,123) in the same way that normal integer ids are obfuscated. Additionally, adds obfuscation to theWcfCommon.csfor itsLocalPathinto theresourceName. TheActionfor WCF is now also checked in cases where it is an empty string and if so theLocalPathis used instead.Reason for change
IDs that contained commas weren't being obfuscated and we weren't doing any obfuscation at all for the WCF integration.
Implementation details
,case withinUriHelpers.IsIdentifierSegmentto treat integer ids with commas as a valid identifierLocalPathwithUriHelpers.GetCleanUriPathwithinWcfCommon.cs- swapping to the cleaned URI seemed to change theresourceNamea decent bit (e.g. would have host/port in it where before it wouldn't)Test coverage
123,123is properly obfuscated now.ActionActionis empty string as well)Other details
AIT-1995
Questions:
123,123with?be a breaking change for customers?