From bb0eae5d3624c44caf61b7a7113337a907146037 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 23 Feb 2021 13:24:00 -0800 Subject: [PATCH 1/3] Fix Windows build of Jaeger tests The Jaeger tests use the low-level syscall package. The Windows specific function called in that package has a different function signature than the unix version. Add a windows specific file using the build flags to isolate this OS specific functionality. --- .../trace/jaeger/assertsocketbuffersize.go | 48 +++++++++++++++++++ .../jaeger/assertsocketbuffersize_windows.go | 39 +++++++++++++++ .../jaeger/reconnecting_udp_client_test.go | 15 ------ 3 files changed, 87 insertions(+), 15 deletions(-) create mode 100644 exporters/trace/jaeger/assertsocketbuffersize.go create mode 100644 exporters/trace/jaeger/assertsocketbuffersize_windows.go diff --git a/exporters/trace/jaeger/assertsocketbuffersize.go b/exporters/trace/jaeger/assertsocketbuffersize.go new file mode 100644 index 000000000000..40f9ce830c74 --- /dev/null +++ b/exporters/trace/jaeger/assertsocketbuffersize.go @@ -0,0 +1,48 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build !windows + +package jaeger + +import ( + "net" + "runtime" + "syscall" + "testing" + + "github.com/stretchr/testify/assert" +) + +func assertSockBufferSize(t *testing.T, expectedBytes int, conn *net.UDPConn) bool { + fd, err := conn.File() + if !assert.NoError(t, err) { + return false + } + + bufferBytes, err := syscall.GetsockoptInt(int(fd.Fd()), syscall.SOL_SOCKET, syscall.SO_SNDBUF) + if !assert.NoError(t, err) { + return false + } + + // The linux kernel doubles SO_SNDBUF value (to allow space for + // bookkeeping overhead) when it is set using setsockopt(2), and this + // doubled value is returned by getsockopt(2) + // https://linux.die.net/man/7/socket + if runtime.GOOS == "linux" { + return assert.GreaterOrEqual(t, expectedBytes*2, bufferBytes) + } + + return assert.Equal(t, expectedBytes, bufferBytes) +} diff --git a/exporters/trace/jaeger/assertsocketbuffersize_windows.go b/exporters/trace/jaeger/assertsocketbuffersize_windows.go new file mode 100644 index 000000000000..285162748d06 --- /dev/null +++ b/exporters/trace/jaeger/assertsocketbuffersize_windows.go @@ -0,0 +1,39 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build windows + +package jaeger + +import ( + "net" + "syscall" + "testing" + + "github.com/stretchr/testify/assert" +) + +func assertSockBufferSize(t *testing.T, expectedBytes int, conn *net.UDPConn) bool { + fd, err := conn.File() + if !assert.NoError(t, err) { + return false + } + + bufferBytes, err := syscall.GetsockoptInt(syscall.Handle(fd.Fd()), syscall.SOL_SOCKET, syscall.SO_SNDBUF) + if !assert.NoError(t, err) { + return false + } + + return assert.Equal(t, expectedBytes, bufferBytes) +} diff --git a/exporters/trace/jaeger/reconnecting_udp_client_test.go b/exporters/trace/jaeger/reconnecting_udp_client_test.go index e68e122743fb..6f4712634acc 100644 --- a/exporters/trace/jaeger/reconnecting_udp_client_test.go +++ b/exporters/trace/jaeger/reconnecting_udp_client_test.go @@ -18,8 +18,6 @@ import ( "fmt" "math/rand" "net" - "runtime" - "syscall" "testing" "time" @@ -82,19 +80,6 @@ func newUDPConn() (net.PacketConn, *net.UDPConn, error) { return mockServer, conn, nil } -func assertSockBufferSize(t *testing.T, expectedBytes int, conn *net.UDPConn) bool { - fd, _ := conn.File() - bufferBytes, _ := syscall.GetsockoptInt(int(fd.Fd()), syscall.SOL_SOCKET, syscall.SO_SNDBUF) - - // The linux kernel doubles SO_SNDBUF value (to allow space for bookkeeping overhead) when it is set using setsockopt(2), and this doubled value is returned by getsockopt(2) - // https://linux.die.net/man/7/socket - if runtime.GOOS == "linux" { - return assert.GreaterOrEqual(t, expectedBytes*2, bufferBytes) - } - - return assert.Equal(t, expectedBytes, bufferBytes) -} - func assertConnWritable(t *testing.T, conn udpConn, serverConn net.PacketConn) { expectedString := "yo this is a test" _, err := conn.Write([]byte(expectedString)) From b354fe752dc05a81548678ce2b9a82ed0be81bee Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 23 Feb 2021 13:33:31 -0800 Subject: [PATCH 2/3] Add changes to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5739741d350..64cfd240a7f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed - The sequential timing check of timestamps in the stdout exporter are now setup explicitly to be sequential (#1571). (#1572) +- Windows build of Jaeger tests now compiles with OS specific functions (#1576). (#1577) ## [0.17.0] - 2020-02-12 From 9e0fcb75d00f6a84559fd7a9589104f9ec6f313d Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 23 Feb 2021 18:40:40 -0800 Subject: [PATCH 3/3] Blind succeed to account for unimplemented functionality on Windows --- .../jaeger/assertsocketbuffersize_windows.go | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/exporters/trace/jaeger/assertsocketbuffersize_windows.go b/exporters/trace/jaeger/assertsocketbuffersize_windows.go index 285162748d06..9fba4ba4d340 100644 --- a/exporters/trace/jaeger/assertsocketbuffersize_windows.go +++ b/exporters/trace/jaeger/assertsocketbuffersize_windows.go @@ -18,22 +18,17 @@ package jaeger import ( "net" - "syscall" "testing" - - "github.com/stretchr/testify/assert" ) func assertSockBufferSize(t *testing.T, expectedBytes int, conn *net.UDPConn) bool { - fd, err := conn.File() - if !assert.NoError(t, err) { - return false - } - - bufferBytes, err := syscall.GetsockoptInt(syscall.Handle(fd.Fd()), syscall.SOL_SOCKET, syscall.SO_SNDBUF) - if !assert.NoError(t, err) { - return false - } - - return assert.Equal(t, expectedBytes, bufferBytes) + // The Windows implementation of the net.UDPConn does not implement the + // functionality to return a file handle, instead a "not supported" error + // is returned: + // + // https://github.com/golang/go/blob/6cc8aa7ece96aca282db19f08aa5c98ed13695d9/src/net/fd_windows.go#L175-L178 + // + // This means we are not able to pass the connection to a syscall and + // determine the buffer size. + return true }