From b9cc0a777123cd6908b325248fdbdb97e815f72c Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Thu, 25 Jun 2026 13:20:33 +0200 Subject: [PATCH 1/4] OSIS-156: log empty bucket list at DEBUG, not as a failure A tenant with no buckets is an expected, recoverable condition: the storage platform answers with a 404/error and getBucketList returns an empty list. OSIS-163 already dropped the full stack trace and lowered the recovery log from ERROR to WARN; this lowers it further to DEBUG with a concise message so the normal 'no buckets yet' case no longer reads as a warning. The empty-list contract is unchanged. Adds a ListAppender regression test asserting the empty-bucket case returns an empty page and is never logged at ERROR or WARN and carries no stack trace. --- .../service/impl/ScalityOsisServiceImpl.java | 6 +- .../impl/ScalityOsisServiceMiscTests.java | 58 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java b/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java index 0475600..f9da5d1 100644 --- a/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java +++ b/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java @@ -1092,7 +1092,11 @@ public PageOfOsisBucketMeta getBucketList(String tenantId, long offset, long lim return pageOfOsisBucketMeta; }); } catch (Exception e) { - logger.warn("Get Bucket List failed; returning empty list: {}", e.getMessage()); + // A tenant with no buckets is an expected, recoverable condition: the platform + // answers with a 404/error and OSIS returns an empty list. Log it at DEBUG with a + // concise message and no stack trace so the normal "no buckets yet" case does not + // read as a failure. A genuine fault still surfaces through the returned empty page. + logger.debug("Get Bucket List returned no buckets; returning empty list: {}", e.getMessage()); // For errors, GetBucketList should return empty PageOfOsisBucketMeta PageInfo pageInfo = new PageInfo(limit, offset); diff --git a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java index 7dcd925..7919910 100644 --- a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java +++ b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java @@ -1,5 +1,9 @@ package com.scality.osis.service.impl; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import com.amazonaws.Response; import com.amazonaws.services.identitymanagement.model.*; import com.amazonaws.services.s3.model.Bucket; @@ -17,6 +21,7 @@ import org.junit.jupiter.api.Test; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; import java.util.ArrayList; @@ -174,6 +179,59 @@ void testGetBucketListErr() { assertEquals(0L, response.getItems().size()); } + /** + * OSIS-156: a tenant that legitimately has no buckets is an expected, recoverable + * condition. The empty-list contract must hold and the recovery must not be logged as + * an error or warning, nor dump a stack trace, so operators do not chase a non-failure. + */ + @Test + void testGetBucketListEmptyLogsAtDebugWithoutErrorOrTrace() { + // Setup: tenant with no buckets surfaces as a 404 from the storage platform + final long offset = 0L; + final long limit = 1000L; + when(s3ClientMock.listBuckets()) + .thenAnswer((Answer>) invocation -> { + throw new S3ServiceException(HttpStatus.NOT_FOUND, "The specified bucket does not exist"); + }); + + final Logger serviceLogger = (Logger) LoggerFactory.getLogger(ScalityOsisServiceImpl.class); + final Level originalLevel = serviceLogger.getLevel(); + // Capture DEBUG so we can assert the expected case is logged there, not at WARN/ERROR. + serviceLogger.setLevel(Level.DEBUG); + final ListAppender appender = new ListAppender<>(); + appender.start(); + serviceLogger.addAppender(appender); + + try { + // Run: empty-list contract must be preserved + final PageOfOsisBucketMeta response = + scalityOsisServiceUnderTest.getBucketList(SAMPLE_TENANT_ID, offset, limit); + + assertEquals(0L, response.getPageInfo().getTotal()); + assertEquals(offset, response.getPageInfo().getOffset()); + assertEquals(limit, response.getPageInfo().getLimit()); + assertEquals(0L, response.getItems().size()); + + // The expected "no buckets" case must never be logged at ERROR or WARN, and must + // not carry a stack trace. + assertTrue(appender.list.stream().noneMatch(e -> e.getLevel() == Level.ERROR), + "the expected empty-bucket case must not be logged at ERROR"); + assertTrue(appender.list.stream().noneMatch(e -> e.getLevel() == Level.WARN), + "the expected empty-bucket case must not be logged at WARN"); + assertTrue(appender.list.stream().allMatch(e -> e.getThrowableProxy() == null), + "the expected empty-bucket case must not dump a stack trace"); + + // The recovery line is emitted, at DEBUG, with the concise message. + assertTrue(appender.list.stream() + .anyMatch(e -> e.getLevel() == Level.DEBUG + && e.getFormattedMessage().contains("returning empty list")), + "the empty-bucket recovery should be logged once at DEBUG"); + } finally { + serviceLogger.detachAppender(appender); + serviceLogger.setLevel(originalLevel); + } + } + // test to check if getUsage API throws an error not implemented // this will be removed as a part of S3C-8266 From 2dd176535127dc360b5ad62d04c32d8455991bc2 Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Thu, 25 Jun 2026 16:05:22 +0200 Subject: [PATCH 2/4] OSIS-156: keep genuine bucket-list faults at WARN, quiet only the expected 404 to DEBUG --- .../service/impl/ScalityOsisServiceImpl.java | 18 +++++--- .../impl/ScalityOsisServiceMiscTests.java | 43 +++++++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java b/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java index f9da5d1..3ff84ce 100644 --- a/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java +++ b/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java @@ -27,6 +27,7 @@ import com.scality.osis.utapiclient.dto.MetricsData; import com.scality.osis.utapiclient.services.UtapiServiceClient; import com.scality.osis.utils.ScalityModelConverter; +import com.scality.osis.s3.impl.S3ServiceException; import com.scality.osis.utils.ScalityUtils; import com.scality.osis.vaultadmin.VaultAdmin; import com.scality.osis.vaultadmin.impl.VaultServiceException; @@ -1092,11 +1093,18 @@ public PageOfOsisBucketMeta getBucketList(String tenantId, long offset, long lim return pageOfOsisBucketMeta; }); } catch (Exception e) { - // A tenant with no buckets is an expected, recoverable condition: the platform - // answers with a 404/error and OSIS returns an empty list. Log it at DEBUG with a - // concise message and no stack trace so the normal "no buckets yet" case does not - // read as a failure. A genuine fault still surfaces through the returned empty page. - logger.debug("Get Bucket List returned no buckets; returning empty list: {}", e.getMessage()); + if (e instanceof S3ServiceException + && ((S3ServiceException) e).getStatus() == HttpStatus.NOT_FOUND) { + // A tenant with no buckets is an expected, recoverable condition: the platform + // answers with a 404 and OSIS returns an empty list. Log it at DEBUG with a + // concise message and no stack trace so the normal "no buckets yet" case does + // not read as a failure. + logger.debug("Get Bucket List returned no buckets; returning empty list: {}", e.getMessage()); + } else { + // A genuine fault (Vault, S3, network, or auth) stays visible to operators at + // WARN, even though the OSE contract still requires returning an empty page here. + logger.warn("Get Bucket List failed; returning empty list: {}", e.getMessage()); + } // For errors, GetBucketList should return empty PageOfOsisBucketMeta PageInfo pageInfo = new PageInfo(limit, offset); diff --git a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java index 7919910..867bbde 100644 --- a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java +++ b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java @@ -232,6 +232,49 @@ void testGetBucketListEmptyLogsAtDebugWithoutErrorOrTrace() { } } + /** + * OSIS-156 (follow-up): only the expected empty-bucket 404 is quieted to DEBUG. A genuine + * fault (here a 500 from the storage platform) must stay visible at WARN so operators are not + * blind to real failures, while the empty-page contract is still honored. + */ + @Test + void testGetBucketListGenuineFaultLogsAtWarn() { + // Setup: a non-404 fault from the storage platform (genuine failure, not "no buckets") + final long offset = 0L; + final long limit = 1000L; + when(s3ClientMock.listBuckets()) + .thenAnswer((Answer>) invocation -> { + throw new S3ServiceException(HttpStatus.INTERNAL_SERVER_ERROR, "storage platform unavailable"); + }); + + final Logger serviceLogger = (Logger) LoggerFactory.getLogger(ScalityOsisServiceImpl.class); + final Level originalLevel = serviceLogger.getLevel(); + serviceLogger.setLevel(Level.DEBUG); + final ListAppender appender = new ListAppender<>(); + appender.start(); + serviceLogger.addAppender(appender); + + try { + // Contract: still an empty page for the caller + final PageOfOsisBucketMeta response = + scalityOsisServiceUnderTest.getBucketList(SAMPLE_TENANT_ID, offset, limit); + assertEquals(0L, response.getItems().size()); + + // A genuine (non-404) fault must surface at WARN, not be hidden at DEBUG. + assertTrue(appender.list.stream() + .anyMatch(e -> e.getLevel() == Level.WARN + && e.getFormattedMessage().contains("failed")), + "a genuine bucket-list fault must be logged at WARN"); + assertTrue(appender.list.stream() + .noneMatch(e -> e.getLevel() == Level.DEBUG + && e.getFormattedMessage().contains("no buckets")), + "a genuine fault must not be misreported as the expected no-buckets case"); + } finally { + serviceLogger.detachAppender(appender); + serviceLogger.setLevel(originalLevel); + } + } + // test to check if getUsage API throws an error not implemented // this will be removed as a part of S3C-8266 From 42e665251aa7bad6b7de4da734c9b12273ff145d Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Thu, 25 Jun 2026 16:14:47 +0200 Subject: [PATCH 3/4] OSIS-156: drop ticket references from test comments --- .../service/impl/ScalityOsisServiceMiscTests.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java index 867bbde..bbb9cd8 100644 --- a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java +++ b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java @@ -180,9 +180,9 @@ void testGetBucketListErr() { } /** - * OSIS-156: a tenant that legitimately has no buckets is an expected, recoverable - * condition. The empty-list contract must hold and the recovery must not be logged as - * an error or warning, nor dump a stack trace, so operators do not chase a non-failure. + * A tenant that legitimately has no buckets is an expected, recoverable condition. The + * empty-list contract must hold and the recovery must not be logged as an error or + * warning, nor dump a stack trace, so operators do not chase a non-failure. */ @Test void testGetBucketListEmptyLogsAtDebugWithoutErrorOrTrace() { @@ -233,9 +233,9 @@ void testGetBucketListEmptyLogsAtDebugWithoutErrorOrTrace() { } /** - * OSIS-156 (follow-up): only the expected empty-bucket 404 is quieted to DEBUG. A genuine - * fault (here a 500 from the storage platform) must stay visible at WARN so operators are not - * blind to real failures, while the empty-page contract is still honored. + * Only the expected empty-bucket 404 is quieted to DEBUG. A genuine fault (here a 500 from + * the storage platform) must stay visible at WARN so operators are not blind to real + * failures, while the empty-page contract is still honored. */ @Test void testGetBucketListGenuineFaultLogsAtWarn() { From e16e8eaf7437b0a4c5bdfc740b1d98e95a88f533 Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Thu, 25 Jun 2026 17:12:31 +0200 Subject: [PATCH 4/4] OSIS-156: classify on AmazonS3Exception (the type AmazonS3.listBuckets throws), not S3ServiceException; mocks now match production --- .../osis/service/impl/ScalityOsisServiceImpl.java | 6 +++--- .../service/impl/ScalityOsisServiceMiscTests.java | 14 ++++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java b/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java index 3ff84ce..9f9e8e5 100644 --- a/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java +++ b/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java @@ -10,6 +10,7 @@ import com.amazonaws.services.identitymanagement.AmazonIdentityManagement; import com.amazonaws.services.identitymanagement.model.*; import com.amazonaws.services.s3.AmazonS3; +import com.amazonaws.services.s3.model.AmazonS3Exception; import com.amazonaws.services.s3.model.Bucket; import com.amazonaws.util.StringUtils; import com.fasterxml.jackson.databind.ObjectMapper; @@ -27,7 +28,6 @@ import com.scality.osis.utapiclient.dto.MetricsData; import com.scality.osis.utapiclient.services.UtapiServiceClient; import com.scality.osis.utils.ScalityModelConverter; -import com.scality.osis.s3.impl.S3ServiceException; import com.scality.osis.utils.ScalityUtils; import com.scality.osis.vaultadmin.VaultAdmin; import com.scality.osis.vaultadmin.impl.VaultServiceException; @@ -1093,8 +1093,8 @@ public PageOfOsisBucketMeta getBucketList(String tenantId, long offset, long lim return pageOfOsisBucketMeta; }); } catch (Exception e) { - if (e instanceof S3ServiceException - && ((S3ServiceException) e).getStatus() == HttpStatus.NOT_FOUND) { + if (e instanceof AmazonS3Exception + && ((AmazonS3Exception) e).getStatusCode() == HttpStatus.NOT_FOUND.value()) { // A tenant with no buckets is an expected, recoverable condition: the platform // answers with a 404 and OSIS returns an empty list. Log it at DEBUG with a // concise message and no stack trace so the normal "no buckets yet" case does diff --git a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java index bbb9cd8..3e46dc8 100644 --- a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java +++ b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java @@ -6,11 +6,11 @@ import ch.qos.logback.core.read.ListAppender; import com.amazonaws.Response; import com.amazonaws.services.identitymanagement.model.*; +import com.amazonaws.services.s3.model.AmazonS3Exception; import com.amazonaws.services.s3.model.Bucket; import com.amazonaws.services.s3.model.Owner; import com.scality.osis.model.*; import com.scality.osis.model.exception.NotImplementedException; -import com.scality.osis.s3.impl.S3ServiceException; import com.scality.osis.utapi.impl.UtapiServiceException; import com.scality.osis.utapiclient.dto.ListMetricsRequestDTO; import com.scality.osis.utapiclient.dto.MetricsData; @@ -166,8 +166,10 @@ void testGetBucketListErr() { final long limit = 1000L; when(s3ClientMock.listBuckets()) .thenAnswer((Answer>) invocation -> { - throw new S3ServiceException(HttpStatus.BAD_REQUEST, + final AmazonS3Exception ex = new AmazonS3Exception( "Requested offset is outside the total available items"); + ex.setStatusCode(HttpStatus.BAD_REQUEST.value()); + throw ex; }); final PageOfOsisBucketMeta response = scalityOsisServiceUnderTest.getBucketList(SAMPLE_TENANT_ID, offset, limit); @@ -191,7 +193,9 @@ void testGetBucketListEmptyLogsAtDebugWithoutErrorOrTrace() { final long limit = 1000L; when(s3ClientMock.listBuckets()) .thenAnswer((Answer>) invocation -> { - throw new S3ServiceException(HttpStatus.NOT_FOUND, "The specified bucket does not exist"); + final AmazonS3Exception ex = new AmazonS3Exception("The specified bucket does not exist"); + ex.setStatusCode(HttpStatus.NOT_FOUND.value()); + throw ex; }); final Logger serviceLogger = (Logger) LoggerFactory.getLogger(ScalityOsisServiceImpl.class); @@ -244,7 +248,9 @@ void testGetBucketListGenuineFaultLogsAtWarn() { final long limit = 1000L; when(s3ClientMock.listBuckets()) .thenAnswer((Answer>) invocation -> { - throw new S3ServiceException(HttpStatus.INTERNAL_SERVER_ERROR, "storage platform unavailable"); + final AmazonS3Exception ex = new AmazonS3Exception("storage platform unavailable"); + ex.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR.value()); + throw ex; }); final Logger serviceLogger = (Logger) LoggerFactory.getLogger(ScalityOsisServiceImpl.class);