#1034: AWS SQS and SNS support

[rypdal]

Changes

• incoming requests (OpenTelemetry.Instrumentation.AWSLambda) : implemented parent extraction for incoming SQS and SNS events (getters)
• outgoing requests (OpenTelemetry.Contrib.Instrumentation.AWS): implemented enrichment of outgoing message attributes by trace data (setters)
• no API changes

[codecov]

Codecov Report

Merging #1051 (8c8b9a0) into main (d3f70ef) will decrease coverage by 0.18%.
The diff coverage is 61.29%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1051      +/-   ##
==========================================
- Coverage   72.83%   72.65%   -0.18%     
==========================================
  Files         247      252       +5     
  Lines        8835     8949     +114     
==========================================
+ Hits         6435     6502      +67     
- Misses       2400     2447      +47     

Impacted Files	Coverage Δ
...ntation.AWSLambda/Implementation/AWSLambdaUtils.cs	78.94% <23.07%> (-9.12%)	⬇️
...tion.AWSLambda/Implementation/AWSMessagingUtils.cs	37.25% <37.25%> (ø)
...on.AWS/Implementation/AWSTracingPipelineHandler.cs	87.75% <57.14%> (-3.55%)	⬇️
...strumentation.AWS/Implementation/AWSServiceType.cs	66.66% <66.66%> (ø)
...tion.AWS/Implementation/SnsRequestContextHelper.cs	95.00% <95.00%> (ø)
...tion.AWS/Implementation/SqsRequestContextHelper.cs	95.00% <95.00%> (ø)
...umentation.AWS/Implementation/AWSMessagingUtils.cs	100.00% <100.00%> (ø)
...rumentation.AWS/Implementation/AWSServiceHelper.cs	100.00% <100.00%> (ø)
...ation.AWSLambda/AWSLambdaInstrumentationOptions.cs	100.00% <100.00%> (ø)
...etry.Instrumentation.AWSLambda/AWSLambdaWrapper.cs	94.64% <100.00%> (+0.09%)	⬆️
... and 1 more

[rypdal]

Hi @srprash ! The final changes have been made after discussions about creating/not creating a parent from a message batch. The new configuration setting has been introduced for toggling the parent creation. Please, review these as well as previous changes. Thank you.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: rypdal / name: Oleksiy Dubinin (8b93219, 7f75e12, c0ff714, 4cc849a, 222f782, 8bdad4d, 1147917, e20c509, ed1ef5a, 2061808, c426560, 165a0d1, 9f8d96a, d55c66b, e51a882, 4a09e75, 3fbb6b0, 6996859, c98d9e9, 933c3be, 94e489c, bdb0077, e88edbe, 5ce87e1, 5bed7c6, 24ae92d, d2613c4, 546ef3f, 0460e9b, 70acd21, 63e7445, 297effc, 9a9fa26, c8bec70)

[Oberon00]

EasyCLA issue seems to be open-telemetry/community#1445

[trask]

/easycla

[srprash]

Looping in @atshaw43 to take a look.

[martinjt]

Spent a bit of time thinking about this and chatting to messaging people about it, and I'm still not completely sure that this is the right approach. That said, I'm also not sure what the right approach should be here, and so, please take this as an observation and in no way a blocker.

The links that are being created could be misleading. I'm behind the fact that we shouldn't set a parent (a batch execution is a trace in itself), however, linking that batch execution to the parent spans of the messages it's processing isn't how I would see this being beneficial.

The issue is that you can't link to the spans that are created for the message processing as, by definition, they're started/created after the batch has started. Which leaves the dilemma of what to do?

I look at things through the lens of someone debugging a platform, rather than the code.

One issue I'm thinking of is that the batch processing crashes out half way through, and you end up with the latter half of the messages not being processed. In that scenario, the latter half of the messages' trace parent is linked to the batch execution, however, nothing happened. That could be really confusing to the person debugging.

The other part is that I would expect the message processing span to be linked to the batch execution span, while having a parent of the originating message. Which means we're ending up with twice the amount of links being created.

I haven't seen anything in the library like a helper method to create an activity from the item in the batch, that would be a nice addition as we would be expecting people to do that in batch processing mode.

All in all, I think we've come along way in the right direction now. I haven't reviewed the code for style or completeness etc. I've assumed someone else was doing that, but if I need to then please ping me.

[martinjt]

I also didn't see anything in a README about this setting, I think that would be a strong suggestion from me as the setting will need a lot of explaining.

[Oberon00]

Are you aware of the messaging semantic convention group's proposal about the messaging parenting/linking topic? open-telemetry/oteps#220
I think the implementation here is now in alignment with that.

[Oberon00]

I haven't seen anything in the library like a helper method to create an activity from the item in the batch, that would be a nice addition as we would be expecting people to do that in batch processing mode.

I agree on this, but that can be provided in a follow-up (this PR is already rather large).