aws / aws-xray-sdk-node

The official AWS X-Ray SDK for Node.js.
Apache License 2.0
271 stars 155 forks source link

Default annotations and metadata for captureAWS() #106

Open darcoli opened 5 years ago

darcoli commented 5 years ago

Typically we use captureAWS(require('aws-sdk')) to patch all aws sdk calls and get xray traces with subsegments for all sdk calls. There are cases where one would want to add default annotations and/or metadata to all these traces but I do not see a way to achieve this.

For example, in our organisation we use separate cloudformation stacks to have isolated development environments within the same aws account. However, in the xray console we cannot restrict by cloudformation stack so it would help if all traces had a default annotation/metadata with the cloudformation stack name (and other details). This would then allow us to filter traces from the xray console using something like annotations.environment = "envABC"

I went through the code and it seems quite doable to change captureAWS(awssdk) to something like: captureAWS(awssdk, defaultAnnotations = {}, defaultMetadata = {})

This would then be called as follows:

let AWS = captureAWS(require('aws-sdk'),
                     {environment: 'envABC', author: 'xyz'},                
                     {description: 'something'}                
          );

What do you think? Any better way to do this?

awssandra commented 5 years ago

Hi Daricoli,

Sounds like a good use case. I'm wondering if instead we could rework the AWS whitelist file to add an annotation qualifier. For metadata (which cannot be filtered), you can simply add the requested service/fields into the whitelist, and the patcher will automatically record it.

https://github.com/aws/aws-xray-sdk-node/blob/master/packages/core/lib/resources/aws_whitelist.json

Whitelist configuration: https://github.com/aws/aws-xray-sdk-node/tree/master/packages/core#aws-sdk-whitelist-configuration

darcoli commented 5 years ago

Hi Sandra Thanks for your prompt reply. Actually I did review the whitelist configuration but from what I can see this only deals aws-sdk calls. I would also like to have this functionality for other patchers such as captureMysql() - the way I see it, the approach I suggested can be adopted for all capture*() calls.

Today I got this to work for captureAWS and captureMysql. I can create pull requests for these two and have the changes reviewed if you agree.

chrisradek commented 5 years ago

@darcoli With regards to your use-case, would setting an annotation on your root segment in your app with your environment information suffice? When you write a filter expression that checks the value of an annotation, you should see a service graph that includes traces that contain any segments with your annotation.

Your idea for captureAWS and captureMysql is interesting. One thing we'll have to think about is if this API can be carried over to the other X-Ray SDKs, or if we should expose another method to add annotations/metadata. For example, we could expose an API that allows you to set an annotation or metadata on a trace, vs forcing you to assign it on a specific segment.

darcoli commented 5 years ago

@chrisradek I am using this from lambda functions and it seems I can only add annotations/metadata on subsegments. Otherwise i get an error: Function "addMetadata" cannot be called on an AWS Lambda segment.

You can review the changes to aws-xray-sdk-core in my branch. Did not push the mysql changes because i am waiting for #98 to be merged ;)

vertisan commented 2 years ago

Any progress about introduce a "global" annotations?

willarmiros commented 2 years ago

Hi @vertisan,

This feature is not available nor is it on our roadmap for the X-Ray SDKs. However, you could accomplish this goal using OpenTelemetry JavaScript. Via the AWS Distro for OpenTelemetry, you can send traces to X-Ray through OpenTelemetry, and add "global" attributes to all traces which can then be converted to annotations. For getting started, see: https://aws-otel.github.io/docs/getting-started/js-sdk/trace-manual-instr