Feature request: Add isTraceSampled method to Tracer

[icholy]

Use case

I'd like a attribute in my logs to indicate whether or not a request has been sampled. Something like this:

{
  "level": "ERROR",
  "msg": "something went wrong",
  "trace_id": "1-6419d889-20172e50462fac2c7c481454",
  "trace_sampled": true
}

I can populate the trace_id using Tracer#getRootXrayTraceId , but there's no way to find out if the trace is sampled.

Solution/User Experience

A new isTraceSampled method on the Tracer class.

log({
  level: "ERROR",
  msg: "something went wrong",
  trace_id: tracer.getRootXrayTraceId(),
  trace_sampled: tracer.isTraceSampled(),
})

Alternative solutions

Implement my own isTraceSampled function which parses the _X_AMZN_TRACE_ID header value.

Acknowledgment

• This feature request meets Lambda Powertools Tenets
• Should this be considered in other Lambda Powertools languages? i.e. Python, Java, and .NET

Future readers

Please react with 👍 and your use case to help us understand customer demand.

[icholy]

I'd be willing to implement this if it's accepted.

[dreamorosi]

Hi @icholy, thank you for taking the time to open this feature request.

I think the idea is valuable and I'm open to consider it.

Before going to the implementation I'd like us to spend some time here in this issue discussing a potential implementation.

Those are some points we should discuss and agree on before moving to a PR:

• Return type of the method, considering that the Trace data might be undefined
• Where/how this new method should be implemented (I see some common logic with this existing method)
• Agree on a name for the method that conveys its meaning
• What type of changes should be made to the documentation to articulate the feature/method

If you're still interested in contributing this function, I'd be happy to collaborate on a proposal.

[icholy]

• Return type of the method, considering that the Trace data might be undefined:
  • boolean | undefined
    • I think we could get away with just boolean. If there's no trace, then it's not sampled.
      • Doesn't matter much to me since if (tracer.isTraceSampled()) { will work either way.
• Where/how this new method should be implemented (I see some common logic with this existing method)
  • Add a EnvironmentVariableService#getXrayTraceSampled method.
  • Use that method inside the Tracer#isTraceSampled
• Agree on a name for the method that conveys its meaning:
  • I feel like isTraceSampled is pretty clear. If you disagree, here are some other options:
    • isSampled
    • isTraceSampled
    • getTraceSampled
    • getRootXrayTraceSampled
• What type of changes should be made to the documentation to articulate the feature/method
  • Not sure

[dreamorosi]

• Return type of the method, considering that the Trace data might be undefined:

  • boolean | undefined

    • I think we could get away with just boolean. If there's no trace, then it's not sampled.
      • Doesn't matter much to me since if (tracer.isTraceSampled()) { will work either way.

I agree, let's stick to boolean only and assume that a missing value equals to false as it means no tracing can happen.

• Where/how this new method should be implemented (I see some common logic with this existing method)
  • Add a EnvironmentVariableService#getXrayTraceSampled method.
  • Use that method inside the Tracer#isTraceSampled

I agree with the second point.

Regarding the first one, we are in a good direction but I would try to make an effort to not have duplicate logic between the existing getXrayTraceSampled and new isXrayTraceSampled method.

The current implementation for the former is:

public getXrayTraceId(): string | undefined {
  const xRayTraceId = this.get(this.xRayTraceIdVariable);

  if (xRayTraceId === '') return undefined;

  return xRayTraceId.split(';')[0].replace('Root=', '');
}

Given that both methods now need to access the tracing header, I would consider extracting part of the logic and reuse it for both, kind of like:

private getXrayTraceData: string[] {
  const xRayTraceData = this.get(this.xRayTraceIdVariable);
  if (xRayTraceData === '') return undefined;

  return xRayTraceData.split(';');
}

public getXrayTraceId(): string | undefined {
  const xRayTraceData = this.getXrayTraceData();

  if (xRayTraceData.length === 0) return undefined;

  return xRayTraceData[0].replace('Root=', '');
}

public isXrayTraceSampled(): boolean {
  const xRayTraceData = this.getXrayTraceData();

  // TODO: implement
}

• Agree on a name for the method that conveys its meaning:
  • I feel like isTraceSampled is pretty clear. If you disagree, here are some other options:
    • isSampled
    • isTraceSampled
    • getTraceSampled
    • getRootXrayTraceSampled

Okay. I agree that isTraceSampled is fairly clear and aligns with the fact that we are expecting a boolean value, so I think we can safely rule out the ones with get.

With that said, I think it would be more consistent to have it called isXrayTraceSampled instead.

• What type of changes should be made to the documentation to articulate the feature/method

  • Not sure

I'm available to help with the editing during the review phase, but the PR for this feature will have to include changes to the corresponding docs as well.

Generally speaking, I think the existing section currently titled "Access AWS X-Ray Root Trace ID" should be changed to something like "Access AWS X-Ray Trace Data", and its content should reflect that:

• Now there are two methods: getRootXrayTraceId() and isXrayTraceSampled()
• Explain that these two methods can be used to access the X-Ray trace data present in the tracing header
• Keep the existing content about getRootXrayTraceId()
• Add a new piece that discusses isXrayTraceSampled(), touches on when it can be useful, shows a piece of code (i.e. similar to the one in your OP)

If you agree with the points/direction, let me know and I'll assign the issue to you and you can start working on a PR.

[icholy]

Sounds good to me.

[dreamorosi]

Hi @icholy, I just wanted to check if you were still working on this feature.

If not, please let me know so I can put it back on the backlog.

[icholy]

@dreamorosi I'm on holiday. I'll be back next week.

[icholy]

Working on this now.

[github-actions]

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

[icholy]

@dreamorosi will there be a release soon?

[dreamorosi]

Yes, I'm planning a release this week. It'll include the feature

[github-actions]

This is now released under v1.9.0 version!