fix: nil-check KeyValue.Value

[lizthegrey]

Which problem is this PR solving?

• somehow we have a corrupt client that's sending a nil Value pointer, which is causing a crash.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0xc124e2]

goroutine 959 [running]:
github.com/honeycombio/refinery/route.addAttributesToMap(0xc06ad49c20, 0xc06a9e80c0, 0x7, 0x8)
	/app/route/otlp_trace.go:236 +0x62
github.com/honeycombio/refinery/route.processTraceRequest(0xf3a018, 0xc06a79b740, 0xc0000d2260, 0xc0694f6b80, 0xc048188f40, 0x20, 0xc04a920b28, 0x4, 0xc0695825a0, 0x0)
	/app/route/otlp_trace.go:125 +0x2292
github.com/honeycombio/refinery/route.(*Router).Export(0xc0000d2260, 0xf3a018, 0xc06a79b740, 0xc0694f6b80, 0xc0000d2260, 0xc06a79b740, 0xc04c063ba0)
	/app/route/otlp_trace.go:59 +0x178
github.com/honeycombio/refinery/internal/opentelemetry-proto-gen/collector/trace/v1._TraceService_Export_Handler(0xde9a00, 0xc0000d2260, 0xf3a018, 0xc06a79b740, 0xc06aae93e0, 0x0, 0xf3a018, 0xc06a79b740, 0xc06ac94000, 0x1c197)
	/app/internal/opentelemetry-proto-gen/collector/trace/v1/trace_service.pb.go:190 +0x214
google.golang.org/grpc.(*Server).processUnaryRPC(0xc00034a700, 0xf416f8, 0xc048262600, 0xc0695825a0, 0xc000407020, 0x1424ee0, 0x0, 0x0, 0x0)
	/go/pkg/mod/google.golang.org/grpc@v1.39.1/server.go:1293 +0x52b
google.golang.org/grpc.(*Server).handleStream(0xc00034a700, 0xf416f8, 0xc048262600, 0xc0695825a0, 0x0)
	/go/pkg/mod/google.golang.org/grpc@v1.39.1/server.go:1618 +0xd0c
google.golang.org/grpc.(*Server).serveStreams.func1.2(0xc04921a1a0, 0xc00034a700, 0xf416f8, 0xc048262600, 0xc0695825a0)
	/go/pkg/mod/google.golang.org/grpc@v1.39.1/server.go:941 +0xab
created by google.golang.org/grpc.(*Server).serveStreams.func1
	/go/pkg/mod/google.golang.org/grpc@v1.39.1/server.go:939 +0x1fd

Short description of the changes

• Skips attributes with no key, or no value.

[akasprzok]

🙏🏼

[irvingpop]

FWIW this is nearly identical to what we have in hound, except there it has a .getKey()

		if attr.GetKey() == "" || attr.Value == nil {

should we make them the same for consistency?

[lizthegrey]

FWIW this is nearly identical to what we have in hound, except there it has a .getKey()

		if attr.GetKey() == "" || attr.Value == nil {

should we make them the same for consistency?

defer decision to telemetry team, I'm just humble oncaller standing in

[MikeGoldsmith]

Thanks for the PR @lizthegrey - we're in the middle of moving refinery to use Husky so I ported the fix there.

@irvingpop Husky is in use in SecureTenancy now, and will be in Refinery soon. I'll be working on adding it to Shepherd next.

[akasprzok]

Thanks for the PR @lizthegrey - we're in the middle of moving refinery to use Husky so I ported the fix there.

@irvingpop Husky is in use in SecureTenancy now, and will be in Refinery soon. I'll be working on adding it to Shepherd next.

@MikeGoldsmith this fix was submitted to help us at BlockFi with a problem we're currently having with refinery.
We were hoping to be able to use this fix today without having to build our own version of refinery.

[lizthegrey]

Thanks for the PR @lizthegrey - we're in the middle of moving refinery to use Husky so I ported the fix there.
@irvingpop Husky is in use in SecureTenancy now, and will be in Refinery soon. I'll be working on adding it to Shepherd next.

@MikeGoldsmith this fix was submitted to help us at BlockFi with a problem we're currently having with refinery. We were hoping to be able to use this fix today without having to build our own version of refinery.

I believe you can still fetch the artifact: https://app.circleci.com/pipelines/github/honeycombio/refinery/946/workflows/e29a9ecd-c508-47ac-9815-e66d92ab562b/jobs/2638/artifacts

[robbkidd]

@akasprzok The same fix has come in via #341, just merged. We're doing a few more pre-release checks and aim to cut a new release today.

[robbkidd]

And released in v1.7.0!