Feature request: Don't panic when a subsegment can't be found

[a-h]

When I run this code on my local machine, I don't expect a panic.

package main

import (
	"fmt"
	"io"
	"os"

	"github.com/aws/aws-xray-sdk-go/xray"
)

func main() {
	c := xray.Client(nil)
	resp, err := c.Get("https://google.com")
	if err != nil {
		fmt.Printf("failed to access Google: %v\n", err)
		os.Exit(1)
	}
	io.Copy(os.Stdout, resp.Body)
}

But it panics!

panic: failed to begin subsegment named 'google.com': segment cannot be found.

I was hunting around in the docs to find out how to suppress this behaviour. I'd like my code to use X-Ray, if it's available, but if it's not, I don't care.

I found that I can set an environment variable, but that's a pain, because I'd even have to set that environment variable to run some unit tests or integration tests locally.

AWS_XRAY_CONTEXT_MISSING=IGNORE_ERROR

As per the Go Proverbs, don't panic please. 😁

Suggestions:

• Change the default behaviour so that it doesn't panic because this is unexpected. If you really must panic, explain in the panic message how to fix or disable it.
• Make it more obvious that this would happen, and provide a way to turn off this behaviour without an environment variable (or document it if there is).

[bhautikpip]

Hey @a-h,

Looks like panic is expected in your code because you're trying to create a subsegment without creating a segment first. Ideally either you should create manual segment using xray.BeginSegment API or maybe wrap your HTTP client around HTTP handler instrumentation (examples can be found in README) then X-Ray Go SDK will automatically creates a segment and then Go SDK will create HTTP subsegment. Yeah you're right you can avoid panic using IGNORE_ERROR option which would just ignore the panic and continue execution of your code.

[a-h]

Thanks @bhautikpip, I understand why it happens, that's not the point of the issue.

I'm saying that it's a footgun that the default behaviour of the library is to panic over nothing.

The current default behaviour is not expected, see https://go.dev/blog/defer-panic-and-recover and https://go.dev/blog/errors-are-values

The convention in the Go libraries is that even when a package uses panic internally, its external API still presents explicit error return values.

i.e. the API should return an error value (or do nothing) rather than panic. It's total overkill to crash the program just because an XRay subsegment can't be created.

[bhautikpip]

Yeah that's fair. Right now xray.BeginSubSegment API would panic if it does not find it's parent segment. To avoid this with the current state of the SDK you can set AWS_XRAY_CONTEXT_MISSING as IGNORE_ERROR or LOG_ERROR. I can mark this ask as an enhancement.

[joerdav]

I'd love to see this SDK follow an error handling pattern that is inline with what the Go community agrees on.