티스토리 뷰

오픈소스에 기여를 이어 가면서 저는 "같은 일을 하는 두 갈래 중 한쪽만 고쳐진 자리"를 자주 봅니다. 이미 누군가 문제를 인지하고 한 곳을 손봤는데, 똑같은 문제를 안고 있는 형제 코드가 그대로 남아 있는 경우입니다. 이런 자리는 대개 리뷰 부담이 작습니다. 고칠 방향을 새로 정할 필요 없이, 이미 머지된 형제의 방식을 그대로 따라가면 되기 때문입니다. 이번에 다룰 기여(#8494)도 그렇게 찾은 것입니다. OpenTelemetry Java의 OTLP exporter 테스트에서, gRPC 쪽은 이미 완화된 단언이 HTTP 쪽에는 그대로 남아 환경에 따라 깨질 수 있던 문제였습니다.

 

먼저 PR 링크를 남겨 둡니다. 실제 변경과 리뷰 과정은 https://github.com/open-telemetry/opentelemetry-java/pull/8494 에서 볼 수 있습니다.

 

연결이 안 되는 주소로 일부러 접속을 시켜 봅니다

OpenTelemetry Java의 OTLP exporter에는 connectTimeout이라는 설정이 있습니다. 텔레메트리를 백엔드로 보낼 때 연결이 지정한 시간 안에 맺어지지 않으면 실패로 처리하는 값입니다. 이 설정이 실제로 동작하는지 확인하려면, 연결이 절대 맺어지지 않는 상황을 만들어 놓고 export가 제때 실패하는지를 봐야 합니다.

 

이 확인을 담당하는 테스트가 AbstractHttpTelemetryExporterTest.connectTimeout()입니다. 테스트는 connectTimeout을 1밀리초로 낮춘 exporter를 만들어, 10.255.255.1처럼 라우팅되지 않는(non-routable) 주소로 export를 보냅니다. 이런 주소는 응답하는 호스트가 없으므로 연결이 끝내 맺어지지 않고, 결국 실패로 떨어집니다. 여기서 테스트가 확인하려는 것은 하나입니다. 기본 connect timeout이 10초인데, export가 그 10초를 다 기다리지 않고 훨씬 일찍 실패해야 한다는 것입니다. 즉 connectTimeout이 실제로 적용되고 있음을 시간으로 증명하는 셈입니다.

 

문제는 이 "훨씬 일찍"을 얼마나 이르게 잡느냐에 있었습니다.

 

1초라는 기준이 환경에 따라 흔들립니다

원래 이 테스트는 export가 1초 안에 실패해야 한다고 단언하고 있었습니다.

// AbstractHttpTelemetryExporterTest.connectTimeout() (수정 전)
// Assert that the export request fails well before the default connect timeout of 10s
assertThat(System.currentTimeMillis() - startTimeMillis)
    .isLessThan(TimeUnit.SECONDS.toMillis(1));

여기서 흔들리는 지점은 라우팅되지 않는 주소로의 연결 실패가 항상 즉시 일어나지는 않는다는 데 있습니다. 운영체제와 네트워크 구성에 따라 이런 연결이 실패로 확정되기까지 1초에서 5초 정도가 걸릴 수 있습니다. 커널이 도달 불가를 어떻게 판단하고 재시도하는지가 환경마다 다르기 때문입니다. 그래서 어떤 환경에서는 이 테스트가 잘 통과하다가, 다른 환경에서는 연결 실패가 1초를 넘겨 단언이 깨지는 일이 생깁니다. 코드가 바뀐 게 아니라 실행 환경 때문에 결과가 갈리는, 이른바 flaky 테스트입니다.

 

형제 테스트는 이미 이 문제를 겪고 고쳤습니다

이 대목에서 저는 같은 확인을 하는 다른 테스트가 있는지부터 봤습니다. OTLP exporter는 크게 HTTP 전송과 gRPC 전송 두 갈래가 있고, 각각의 테스트 인프라도 나란히 있습니다. HTTP 쪽 베이스가 AbstractHttpTelemetryExporterTest라면, gRPC 쪽 베이스는 AbstractGrpcTelemetryExporterTest입니다. 둘 다 같은 방식으로 라우팅되지 않는 주소에 접속을 시도해 connectTimeout을 검증합니다.

 

그런데 gRPC 쪽 베이스를 열어 보니, 같은 단언이 이미 6초로 완화되어 있었습니다.

// AbstractGrpcTelemetryExporterTest.connectTimeout() (이미 완화된 상태)
// Assert that the export request fails well before the default connect timeout of 10s
// Note: Connection failures to non-routable IPs can take 1-5 seconds depending on OS/network
assertThat(System.currentTimeMillis() - startTimeMillis)
    .isLessThan(TimeUnit.SECONDS.toMillis(6));

 

이 완화는 PR #7840에서 이루어졌습니다. 그 PR은 원래 다른 목적(OkHttp gRPC sender의 종료 처리)을 다뤘지만, 작업 도중 이 단언이 환경에 따라 깨지는 것을 확인하고 임계값을 1초에서 6초로 늘리면서 위와 같은 설명 주석을 함께 남겼습니다. "라우팅되지 않는 IP로의 연결 실패는 OS와 네트워크에 따라 1~5초가 걸릴 수 있다"는 주석입니다. 즉 이 문제는 이미 한 번 인지되고 문서화된 상태였습니다. 다만 그 수정이 gRPC 베이스 한 곳에만 적용되고, 같은 단언을 가진 HTTP 베이스는 함께 손대지 않은 채 남아 있었습니다.

 

두 테스트가 겪는 상황은 프로토콜과 무관합니다. 연결 실패가 얼마나 걸리느냐는 HTTP냐 gRPC냐가 아니라 운영체제가 도달 불가 주소를 어떻게 처리하느냐에 달려 있기 때문입니다. 그래서 gRPC에서 확인된 흔들림은 HTTP에도 똑같이 적용됩니다. 결국 이 건은 새로운 결함을 발견한 것이라기보다, 이미 내려진 판단이 형제 한쪽에 미처 반영되지 않은 자리를 메우는 일이었습니다.

 

수정은 이미 정해진 방식을 그대로 따라가는 것

고칠 방향을 새로 고민할 필요는 없었습니다. gRPC 베이스가 이미 답을 보여 주고 있었기 때문입니다. HTTP 베이스에도 같은 설명 주석을 한 줄 덧붙이고, 단언의 임계값을 1초에서 6초로 바꾸는 것으로 끝났습니다.

// AbstractHttpTelemetryExporterTest.connectTimeout() (수정 후)
// Assert that the export request fails well before the default connect timeout of 10s
// Note: Connection failures to non-routable IPs can take 1-5 seconds depending on OS/network
assertThat(System.currentTimeMillis() - startTimeMillis)
    .isLessThan(TimeUnit.SECONDS.toMillis(6));

 

여기서 스스로 한 번 멈춰서 확인한 게 있습니다. 단언을 1초에서 6초로 늘리는 것은 검사를 느슨하게 만드는 변경입니다. 그러면 "이렇게 완화해도 이 테스트가 원래 확인하려던 걸 여전히 확인하는가"를 짚어야 합니다. 이 테스트의 목적은 export가 기본 connect timeout인 10초를 다 기다리지 않고 그보다 훨씬 일찍 실패하는지 보는 것입니다. 6초는 10초보다 충분히 작으므로, connectTimeout이 적용되고 있다는 사실은 6초 기준으로도 그대로 증명됩니다. 즉 이 변경은 테스트가 검사하는 의미를 바꾸지 않고, 환경에 따른 시간 변동만 흡수하도록 여유를 준 것입니다. 이미 머지된 gRPC 쪽이 같은 트레이드오프를 받아들였다는 점도 판단에 힘을 실어 주었습니다.

 

이 단언이 어디서 실제로 실행되는지 확인해야 했습니다

이번 기여에서 가장 신경 쓴 부분은 코드 수정 자체가 아니라 검증이었습니다. 제가 고친 AbstractHttpTelemetryExporterTest는 이름 그대로 추상 베이스 클래스이고, 이 모듈에는 실행 가능한 자체 테스트가 들어 있지 않습니다. 그래서 :exporters:otlp:testing-internal 모듈만 테스트해서는 이 단언이 한 번도 실행되지 않습니다. 여기서 멈추면 "테스트를 고쳤는데 그 테스트가 도는지 확인하지 않은" 상태가 됩니다.

 

실제로 이 단언을 실행하는 것은 이 베이스를 상속한 구체 클래스들입니다. 그 클래스들은 :exporters:otlp:all 모듈 아래, 전송 방식별로 나뉜 별도의 테스트 소스셋에 있습니다. OkHttp 계열 sender를 쓰는 쪽과 JDK의 HTTP sender를 쓰는 쪽이 나뉘어 있고, 여기에 span·metric·log 세 신호가 곱해져 connectTimeout() 케이스가 여섯 벌 존재합니다. 이들은 평범한 test 태스크가 아니라 각자의 테스트 스위트 태스크로 실행됩니다. 그래서 저는 :exporters:otlp:testing-internal의 검사를 돌려 포맷과 컴파일을 확인한 뒤, :exporters:otlp:all의 해당 스위트 태스크들을 직접 돌려 여섯 개 connectTimeout() 케이스가 모두 통과하는지 확인했습니다. 베이스만 보고 통과라고 적지 않고, 그 베이스가 실제로 실행되는 자리를 찾아 돌려 본 셈입니다.

 

손대는 자리가 어디에 속하는지 보는 습관

이 저장소에서는 stable 아티팩트의 공개 API를 바꾸면 japicmp가 빌드를 막습니다. 그래서 변경이 공개 표면에 닿는지부터 가늠하는 습관이 필요합니다. 이번에 손댄 코드는 io.opentelemetry.exporter.otlp.testing.internal 패키지 안에 있습니다. 이 모듈은 테스트끼리 공유하는 인프라라 아티팩트로 배포되지 않고, 그래서 japicmp 검사를 받는 대상 자체가 아닙니다. 패키지 이름의 internal 역시 같은 방향을 가리킵니다. 이 저장소에서 internal 패키지는 공개 API 호환성 보장에서 빠집니다. 바꾼 것도 테스트 단언의 임계값 한 줄과 설명 주석 한 줄뿐이라 어떤 시그니처도 건드리지 않았습니다. 그래서 docs/apidiffs 아래에 커밋할 diff가 생기지 않았습니다. CHANGELOG.md는 PR에서 직접 항목을 넣지 않고 릴리스 때 정리되는데, 이번 변경은 사용자에게 보이는 동작을 바꾸지 않았으니 거기에 실릴 내용도 아닙니다.

 

이 성격을 분명히 해 두는 게 중요하다고 느꼈습니다. 이번 변경은 눈에 보이는 장애를 고친 것이 아니라, 환경에 따라 간헐적으로 깨지던 테스트를 안정화한 것입니다. 프로덕션 동작은 그대로이고, 테스트가 검사하는 의미도 그대로입니다. 그래서 PR 본문에도 이 점을 부풀리지 않고, 형제 gRPC 베이스에 이미 적용된 완화를 HTTP 베이스에 맞추는 보완이라고 담백하게 적었습니다. 실제로 일어나는 일과 다르게 과장해서 쓰면 리뷰어가 검증하기 더 어려워지기 때문입니다.

 

남은 것

돌아보면 이번 기여에서 코드를 바꾸는 일은 두 줄이면 끝났습니다. 정작 시간이 걸린 것은 그 두 줄이 정말 타당한지 확인하는 과정이었습니다. gRPC 쪽이 왜 완화됐는지 이력을 따라가 확인하고, 같은 흔들림이 HTTP에도 적용되는지 따져 보고, 단언을 느슨하게 해도 원래 검사하려던 의미가 남는지 짚고, 마지막으로 이 단언이 실제로 실행되는 자리를 찾아 돌려 보는 일들이었습니다.

 

테스트를 느슨하게 만드는 변경은 조심스럽게 다뤄야 한다는 걸 다시 느꼈습니다. 임계값을 늘리는 것만 보면 검사를 약하게 만드는 것처럼 보이지만, 그 테스트가 무엇을 증명하려는지를 기준으로 보면 오히려 잘못된 실패를 걷어 내는 일일 수 있습니다. 무엇을 확인하려는 테스트인지 먼저 분명히 해 두면, 임계값을 어디까지 열어도 되는지도 자연스럽게 따라 나옵니다. 앞으로도 무언가를 새로 만들기 전에, 이미 있는 코드와 이미 내려진 판단이 스스로 알려 주는 자리부터 살펴보려 합니다.