You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Trace Annotations not getting added if Annotation Key has a space in it #1499
I noticed that Trace Annotations not getting added if Annotation Key has a space in it
The following code works
TracingUtils.putAnnotation("TraceName", "MyTrace");
The following code doesn't work
TracingUtils.putAnnotation("Trace Name", "MyTrace");
Expected Behavior
Annotation should be visible in Trace details page
Annotation should be visible in Trace details page irrespective of how the key is created.
Current Behavior
Annotation is totally missing if key has a space in it
It looks like annotation keys have to be alphanumeric without spaces for X-Ray. I'm not sure what we should do here - the underlying x-ray library isn't validating this - should we? What do you think?
The confusion arises when space is allowed for metadata key and not for annotations. If currently space cannot be supported, I would suggest updating the powertools documentation to make it clear.
@msailes ACK your point about improving user XP. I feel like it belongs primarily in x-ray and will raise a ticket over there also.
If PT were to validate it, the result would likely be a runtime exception. At least then you'd have a some immediate indication of the problem, rather than a silent failure and a bug hunt. What do you both think about this?
If PT were to validate it, the result would likely be a runtime exception. At least then you'd have some immediate indication of the problem, rather than a silent failure and a bug hunt. What do you both think about this?
If the X-Ray SDK for Java doesn't address this, I completely agree that Powertools should improve this experience. But I don't think this should raise a runtime exception. In general, I believe we shouldn't raise a Runtime error when talking about observability. It is better to emit a warning so that the customer catches this warning and can work to fix the problem with the minimum of data loss - just an annotation in this case.
Activity
thetuxwhocodes commentedon Oct 31, 2023
Steps to Reproduce (for bugs)
scottgerring commentedon Nov 2, 2023
Hey @thetuxwhocodes , thanks for raising the issue!
It looks like annotation keys have to be alphanumeric without spaces for X-Ray. I'm not sure what we should do here - the underlying x-ray library isn't validating this - should we? What do you think?
msailes commentedon Nov 2, 2023
I feel like the X-Ray library should validate this. Powertools also has the chance to help customers avoid this problem.
thetuxwhocodes commentedon Nov 7, 2023
The confusion arises when space is allowed for metadata key and not for annotations. If currently space cannot be supported, I would suggest updating the powertools documentation to make it clear.
scottgerring commentedon Nov 7, 2023
@msailes ACK your point about improving user XP. I feel like it belongs primarily in x-ray and will raise a ticket over there also.
If PT were to validate it, the result would likely be a runtime exception. At least then you'd have a some immediate indication of the problem, rather than a silent failure and a bug hunt. What do you both think about this?
leandrodamascena commentedon Nov 7, 2023
Hey Java folks!!
The X-Ray SDK for Python emits a warning if some unsupported characters are found - https://github.com/aws/aws-xray-sdk-python/blob/9e6fa9f5aed3e36ec1590172f296a2c2c8666608/aws_xray_sdk/core/models/entity.py#L151
If the X-Ray SDK for Java doesn't address this, I completely agree that Powertools should improve this experience. But I don't think this should raise a runtime exception. In general, I believe we shouldn't raise a Runtime error when talking about observability. It is better to emit a warning so that the customer catches this warning and can work to fix the problem with the minimum of data loss - just an annotation in this case.
jeromevdl commentedon Nov 7, 2023
+1 on the log warn, rather than an exception. it also makes sense to have the same behaviour as python.
thetuxwhocodes commentedon Nov 8, 2023
+1 for warning. Runtime exception for monitoring will be a bad user experience.
jdoherty commentedon Nov 8, 2023
Hello - I'll take a look at this :-)
jeromevdl commentedon Nov 14, 2023
fixed, will be released after tomorrow
github-actions commentedon Nov 16, 2023
This is now released under 1.18.0 version!