diff --git a/atlasdb-client/src/main/java/com/palantir/atlasdb/keyvalue/impl/TracingKeyValueService.java b/atlasdb-client/src/main/java/com/palantir/atlasdb/keyvalue/impl/TracingKeyValueService.java index c18f01a612b..785df79280b 100644 --- a/atlasdb-client/src/main/java/com/palantir/atlasdb/keyvalue/impl/TracingKeyValueService.java +++ b/atlasdb-client/src/main/java/com/palantir/atlasdb/keyvalue/impl/TracingKeyValueService.java @@ -41,12 +41,13 @@ import com.palantir.atlasdb.keyvalue.api.RowResult; import com.palantir.atlasdb.keyvalue.api.TableReference; import com.palantir.atlasdb.keyvalue.api.Value; +import com.palantir.atlasdb.logging.LoggingArgs; import com.palantir.atlasdb.tracing.CloseableTrace; import com.palantir.common.base.ClosableIterator; import com.palantir.util.paging.TokenBackedBasicResultsPage; /** - * Wraps a {@link KeyValueService}'s methods with {@link com.palantir.remoting2.tracing.Tracer} + * Wraps a {@link KeyValueService}'s methods with {@link com.palantir.remoting3.tracing.Tracer} * instrumentation. */ public final class TracingKeyValueService extends ForwardingObject implements KeyValueService { @@ -76,7 +77,7 @@ protected KeyValueService delegate() { public void addGarbageCollectionSentinelValues(TableReference tableRef, Iterable cells) { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("addGarbageCollectionSentinelValues({}, {} cells)", - tableRef, Iterables.size(cells))) { + LoggingArgs.safeTableOrPlaceholder(tableRef), Iterables.size(cells))) { delegate().addGarbageCollectionSentinelValues(tableRef, cells); } } @@ -84,8 +85,8 @@ public void addGarbageCollectionSentinelValues(TableReference tableRef, Iterable @Override public void checkAndSet(CheckAndSetRequest checkAndSetRequest) throws CheckAndSetException { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("checkAndSet({}, {})", - checkAndSetRequest.table(), checkAndSetRequest.cell())) { + try (CloseableTrace trace = startLocalTrace("checkAndSet({})", + LoggingArgs.safeTableOrPlaceholder(checkAndSetRequest.table()))) { delegate().checkAndSet(checkAndSetRequest); } } @@ -101,7 +102,8 @@ public void close() { @Override public void compactInternally(TableReference tableRef) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("compactInternally({})", tableRef)) { + try (CloseableTrace trace = startLocalTrace("compactInternally({})", + LoggingArgs.safeTableOrPlaceholder(tableRef))) { delegate().compactInternally(tableRef); } } @@ -122,7 +124,8 @@ public boolean isInitialized() { @Override public void createTable(TableReference tableRef, byte[] tableMetadata) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("createTable({})", tableRef)) { + try (CloseableTrace trace = startLocalTrace("createTable({})", + LoggingArgs.safeTableOrPlaceholder(tableRef))) { delegate().createTable(tableRef, tableMetadata); } } @@ -131,7 +134,7 @@ public void createTable(TableReference tableRef, byte[] tableMetadata) { public void createTables(Map tableNamesToTableMetadata) { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("createTables({})", - tableNamesToTableMetadata.keySet())) { + LoggingArgs.safeTablesOrPlaceholder(tableNamesToTableMetadata.keySet()))) { delegate().createTables(tableNamesToTableMetadata); } } @@ -139,7 +142,8 @@ public void createTables(Map tableNamesToTableMetadata) @Override public void delete(TableReference tableRef, Multimap keys) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("delete({}, {} keys)", tableRef, keys.size())) { + try (CloseableTrace trace = startLocalTrace("delete({}, {} keys)", + LoggingArgs.safeTableOrPlaceholder(tableRef), keys.size())) { delegate().delete(tableRef, keys); } } @@ -147,7 +151,8 @@ public void delete(TableReference tableRef, Multimap keys) { @Override public void deleteRange(TableReference tableRef, RangeRequest range) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("deleteRange({})", tableRef)) { + try (CloseableTrace trace = startLocalTrace("deleteRange({})", + LoggingArgs.safeTableOrPlaceholder(tableRef))) { delegate().deleteRange(tableRef, range); } } @@ -155,7 +160,8 @@ public void deleteRange(TableReference tableRef, RangeRequest range) { @Override public void dropTable(TableReference tableRef) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("dropTable({})", tableRef)) { + try (CloseableTrace trace = startLocalTrace("dropTable({})", + LoggingArgs.safeTableOrPlaceholder(tableRef))) { delegate().dropTable(tableRef); } } @@ -163,7 +169,8 @@ public void dropTable(TableReference tableRef) { @Override public void dropTables(Set tableRefs) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("dropTables({})", tableRefs)) { + try (CloseableTrace trace = startLocalTrace("dropTables({})", + LoggingArgs.safeTablesOrPlaceholder(tableRefs))) { delegate().dropTables(tableRefs); } } @@ -172,7 +179,7 @@ public void dropTables(Set tableRefs) { public Map get(TableReference tableRef, Map timestampByCell) { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("get({}, {} cells)", - tableRef, timestampByCell.size())) { + LoggingArgs.safeTableOrPlaceholder(tableRef), timestampByCell.size())) { return delegate().get(tableRef, timestampByCell); } } @@ -189,7 +196,7 @@ public Set getAllTableNames() { public Multimap getAllTimestamps(TableReference tableRef, Set keys, long timestamp) { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("getAllTimestamps({}, {} keys, ts {})", - tableRef, keys.size(), timestamp)) { + LoggingArgs.safeTableOrPlaceholder(tableRef), keys.size(), timestamp)) { return delegate().getAllTimestamps(tableRef, keys, timestamp); } } @@ -209,7 +216,7 @@ public Map, byte[]>> long timestamp) { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("getFirstBatchForRanges({}, {} ranges, ts {})", - tableRef, Iterables.size(rangeRequests), timestamp)) { + LoggingArgs.safeTableOrPlaceholder(tableRef), Iterables.size(rangeRequests), timestamp)) { return delegate().getFirstBatchForRanges(tableRef, rangeRequests, timestamp); } } @@ -219,7 +226,7 @@ public Map getLatestTimestamps(TableReference tableRef, Map timestampByCell) { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("getLatestTimestamps({}, {} cells)", - tableRef, timestampByCell.size())) { + LoggingArgs.safeTableOrPlaceholder(tableRef), timestampByCell.size())) { return delegate().getLatestTimestamps(tableRef, timestampByCell); } } @@ -227,7 +234,8 @@ public Map getLatestTimestamps(TableReference tableRef, @Override public byte[] getMetadataForTable(TableReference tableRef) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("getMetadataForTable({})", tableRef)) { + try (CloseableTrace trace = startLocalTrace("getMetadataForTable({})", + LoggingArgs.safeTableOrPlaceholder(tableRef))) { return delegate().getMetadataForTable(tableRef); } } @@ -244,32 +252,23 @@ public Map getMetadataForTables() { public ClosableIterator> getRange(TableReference tableRef, RangeRequest rangeRequest, long timestamp) { - //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("getRange({}, ts {})", - tableRef, timestamp)) { - return delegate().getRange(tableRef, rangeRequest, timestamp); - } + // No tracing, as we just return a lazy iterator and don't perform any calls to the backing KVS. + return delegate().getRange(tableRef, rangeRequest, timestamp); } @Override public ClosableIterator>> getRangeOfTimestamps(TableReference tableRef, RangeRequest rangeRequest, long timestamp) { - //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("getRangeOfTimestamps({}, ts {})", - tableRef, timestamp)) { - return delegate().getRangeOfTimestamps(tableRef, rangeRequest, timestamp); - } + // No tracing, as we just return a lazy iterator and don't perform any calls to the backing KVS. + return delegate().getRangeOfTimestamps(tableRef, rangeRequest, timestamp); } @Override public ClosableIterator> getCandidateCellsForSweeping(TableReference tableRef, CandidateCellForSweepingRequest request) { - //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("getCandidateCellsForSweeping({}, ts {})", - tableRef, request.maxTimestampExclusive())) { - return delegate().getCandidateCellsForSweeping(tableRef, request); - } + // No tracing, as we just return a lazy iterator and don't perform any calls to the backing KVS. + return delegate().getCandidateCellsForSweeping(tableRef, request); } @Override @@ -279,7 +278,7 @@ public Map getRows(TableReference tableRef, long timestamp) { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("getRows({}, {} rows, ts {})", - tableRef, Iterables.size(rows), timestamp)) { + LoggingArgs.safeTableOrPlaceholder(tableRef), Iterables.size(rows), timestamp)) { return delegate().getRows(tableRef, rows, columnSelection, timestamp); } } @@ -291,7 +290,7 @@ public Map getRowsColumnRange(TableReference tab long timestamp) { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("getRowsColumnRange({}, {} rows, ts {})", - tableRef, Iterables.size(rows), timestamp)) { + LoggingArgs.safeTableOrPlaceholder(tableRef), Iterables.size(rows), timestamp)) { return delegate().getRowsColumnRange(tableRef, rows, columnRangeSelection, timestamp); } } @@ -302,11 +301,8 @@ public RowColumnRangeIterator getRowsColumnRange(TableReference tableRef, ColumnRangeSelection columnRangeSelection, int cellBatchHint, long timestamp) { - //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("getRowsColumnRange({}, {} rows, {} hint, ts {})", - tableRef, Iterables.size(rows), cellBatchHint, timestamp)) { - return delegate().getRowsColumnRange(tableRef, rows, columnRangeSelection, cellBatchHint, timestamp); - } + // No tracing, as we just return a lazy iterator and don't perform any calls to the backing KVS. + return delegate().getRowsColumnRange(tableRef, rows, columnRangeSelection, cellBatchHint, timestamp); } @Override @@ -323,7 +319,7 @@ public void multiPut(Map> valuesByTa public void put(TableReference tableRef, Map values, long timestamp) { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("put({}, {} values, ts {})", - tableRef, values.size(), timestamp)) { + LoggingArgs.safeTableOrPlaceholder(tableRef), values.size(), timestamp)) { delegate().put(tableRef, values, timestamp); } } @@ -332,7 +328,7 @@ public void put(TableReference tableRef, Map values, long timestam public void putMetadataForTable(TableReference tableRef, byte[] metadata) { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("putMetadataForTable({}, {} bytes)", - tableRef, (metadata == null) ? 0 : metadata.length)) { + LoggingArgs.safeTableOrPlaceholder(tableRef), (metadata == null) ? 0 : metadata.length)) { delegate().putMetadataForTable(tableRef, metadata); } } @@ -340,7 +336,8 @@ public void putMetadataForTable(TableReference tableRef, byte[] metadata) { @Override public void putMetadataForTables(Map tableRefToMetadata) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("putMetadataForTables({})", tableRefToMetadata.keySet())) { + try (CloseableTrace trace = startLocalTrace("putMetadataForTables({})", + LoggingArgs.safeTablesOrPlaceholder(tableRefToMetadata.keySet()))) { delegate().putMetadataForTables(tableRefToMetadata); } } @@ -350,7 +347,7 @@ public void putUnlessExists(TableReference tableRef, Map values) throws KeyAlreadyExistsException { //noinspection unused - try-with-resources closes trace try (CloseableTrace trace = startLocalTrace("putUnlessExists({}, {} values)", - tableRef, values.size())) { + LoggingArgs.safeTableOrPlaceholder(tableRef), values.size())) { delegate().putUnlessExists(tableRef, values); } } @@ -363,8 +360,8 @@ public boolean supportsCheckAndSet() { @Override public void putWithTimestamps(TableReference tableRef, Multimap values) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace( - "putWithTimestamps({}, {} values)", tableRef, values.size())) { + try (CloseableTrace trace = startLocalTrace("putWithTimestamps({}, {} values)", + LoggingArgs.safeTableOrPlaceholder(tableRef), values.size())) { delegate().putWithTimestamps(tableRef, values); } } @@ -372,7 +369,8 @@ public void putWithTimestamps(TableReference tableRef, Multimap val @Override public void truncateTable(TableReference tableRef) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("truncateTable({})", tableRef)) { + try (CloseableTrace trace = startLocalTrace("truncateTable({})", + LoggingArgs.safeTableOrPlaceholder(tableRef))) { delegate().truncateTable(tableRef); } } @@ -380,10 +378,10 @@ public void truncateTable(TableReference tableRef) { @Override public void truncateTables(Set tableRefs) { //noinspection unused - try-with-resources closes trace - try (CloseableTrace trace = startLocalTrace("truncateTables({})", tableRefs)) { + try (CloseableTrace trace = startLocalTrace("truncateTables({})", + LoggingArgs.safeTablesOrPlaceholder(tableRefs))) { delegate().truncateTables(tableRefs); } } - } diff --git a/atlasdb-client/src/main/java/com/palantir/atlasdb/logging/LoggingArgs.java b/atlasdb-client/src/main/java/com/palantir/atlasdb/logging/LoggingArgs.java index 3a593e54b3a..bd8ed1d9dcc 100644 --- a/atlasdb-client/src/main/java/com/palantir/atlasdb/logging/LoggingArgs.java +++ b/atlasdb-client/src/main/java/com/palantir/atlasdb/logging/LoggingArgs.java @@ -43,6 +43,11 @@ * Always returns unsafe, until hydrated. */ public final class LoggingArgs { + + @VisibleForTesting + static final TableReference PLACEHOLDER_TABLE_REFERENCE = + TableReference.createWithEmptyNamespace("{table}"); + @Value.Immutable public interface SafeAndUnsafeTableReferences { SafeArg> safeTableRefs(); @@ -82,6 +87,11 @@ public static SafeAndUnsafeTableReferences tableRefs(Collection .build(); } + public static Iterable safeTablesOrPlaceholder(Collection tables) { + //noinspection StaticPseudoFunctionalStyleMethod - Use lazy iterator. + return Iterables.transform(tables, LoggingArgs::safeTableOrPlaceholder); + } + /** * Returns a safe or unsafe arg corresponding to the supplied table reference, with name "tableRef". */ @@ -89,6 +99,17 @@ public static Arg tableRef(TableReference tableReference) { return tableRef("tableRef", tableReference); } + /** + * If table is safe, returns the table. If unsafe, returns a placeholder. + */ + public static TableReference safeTableOrPlaceholder(TableReference tableReference) { + if (logArbitrator.isTableReferenceSafe(tableReference)) { + return tableReference; + } else { + return PLACEHOLDER_TABLE_REFERENCE; + } + } + public static Arg tableRef(String argName, TableReference tableReference) { return getArg(argName, tableReference.toString(), logArbitrator.isTableReferenceSafe(tableReference)); } diff --git a/atlasdb-client/src/test/java/com/palantir/atlasdb/keyvalue/impl/TracingKeyValueServiceTest.java b/atlasdb-client/src/test/java/com/palantir/atlasdb/keyvalue/impl/TracingKeyValueServiceTest.java index aba8879d187..aeb1b3fa5c3 100644 --- a/atlasdb-client/src/test/java/com/palantir/atlasdb/keyvalue/impl/TracingKeyValueServiceTest.java +++ b/atlasdb-client/src/test/java/com/palantir/atlasdb/keyvalue/impl/TracingKeyValueServiceTest.java @@ -48,7 +48,6 @@ import com.palantir.atlasdb.keyvalue.api.BatchColumnRangeSelection; import com.palantir.atlasdb.keyvalue.api.Cell; import com.palantir.atlasdb.keyvalue.api.CheckAndSetRequest; -import com.palantir.atlasdb.keyvalue.api.ColumnRangeSelection; import com.palantir.atlasdb.keyvalue.api.ColumnSelection; import com.palantir.atlasdb.keyvalue.api.KeyValueService; import com.palantir.atlasdb.keyvalue.api.Namespace; @@ -58,8 +57,6 @@ import com.palantir.atlasdb.keyvalue.api.TableReference; import com.palantir.atlasdb.keyvalue.api.Value; import com.palantir.atlasdb.tracing.TestSpanObserver; -import com.palantir.common.base.ClosableIterator; -import com.palantir.common.base.ClosableIterators; import com.palantir.remoting.api.tracing.SpanType; import com.palantir.remoting3.tracing.Tracer; import com.palantir.util.paging.TokenBackedBasicResultsPage; @@ -120,7 +117,7 @@ public void addGarbageCollectionSentinelValues() throws Exception { ImmutableSet cells = ImmutableSet.of(CELL); kvs.addGarbageCollectionSentinelValues(TABLE_REF, cells); - checkSpan("atlasdb-kvs.addGarbageCollectionSentinelValues(test.testTable, 1 cells)"); + checkSpan("atlasdb-kvs.addGarbageCollectionSentinelValues({table}, 1 cells)"); verify(delegate).addGarbageCollectionSentinelValues(TABLE_REF, cells); verifyNoMoreInteractions(delegate); } @@ -130,7 +127,7 @@ public void checkAndSet() throws Exception { CheckAndSetRequest request = CheckAndSetRequest.singleCell(TABLE_REF, CELL, ROW_NAME, ROW_NAME); kvs.checkAndSet(request); - checkSpan("atlasdb-kvs.checkAndSet(test.testTable, Cell{rowName=726f77, columnName=636f6c, no TTL})"); + checkSpan("atlasdb-kvs.checkAndSet({table})"); verify(delegate).checkAndSet(request); verifyNoMoreInteractions(delegate); } @@ -148,7 +145,7 @@ public void close() throws Exception { public void compactInternally() throws Exception { kvs.compactInternally(TABLE_REF); - checkSpan("atlasdb-kvs.compactInternally(test.testTable)"); + checkSpan("atlasdb-kvs.compactInternally({table})"); verify(delegate).compactInternally(TABLE_REF); verifyNoMoreInteractions(delegate); } @@ -158,7 +155,7 @@ public void createTable() throws Exception { byte[] metadata = new byte[0]; kvs.createTable(TABLE_REF, metadata); - checkSpan("atlasdb-kvs.createTable(test.testTable)"); + checkSpan("atlasdb-kvs.createTable({table})"); verify(delegate).createTable(TABLE_REF, metadata); verifyNoMoreInteractions(delegate); } @@ -168,7 +165,7 @@ public void createTables() throws Exception { byte[] metadata = new byte[0]; kvs.createTables(ImmutableMap.of(TABLE_REF, metadata)); - checkSpan("atlasdb-kvs.createTables([test.testTable])"); + checkSpan("atlasdb-kvs.createTables([{table}])"); verify(delegate).createTables(ImmutableMap.of(TABLE_REF, metadata)); verifyNoMoreInteractions(delegate); } @@ -178,7 +175,7 @@ public void delete() throws Exception { Multimap cells = ImmutableMultimap.of(CELL, 1L); kvs.delete(TABLE_REF, cells); - checkSpan("atlasdb-kvs.delete(test.testTable, 1 keys)"); + checkSpan("atlasdb-kvs.delete({table}, 1 keys)"); verify(delegate).delete(TABLE_REF, cells); verifyNoMoreInteractions(delegate); } @@ -187,7 +184,7 @@ public void delete() throws Exception { public void dropTable() throws Exception { kvs.dropTable(TABLE_REF); - checkSpan("atlasdb-kvs.dropTable(test.testTable)"); + checkSpan("atlasdb-kvs.dropTable({table})"); verify(delegate).dropTable(TABLE_REF); verifyNoMoreInteractions(delegate); } @@ -196,7 +193,7 @@ public void dropTable() throws Exception { public void dropTables() throws Exception { kvs.dropTables(ImmutableSet.of(TABLE_REF)); - checkSpan("atlasdb-kvs.dropTables([test.testTable])"); + checkSpan("atlasdb-kvs.dropTables([{table}])"); verify(delegate).dropTables(ImmutableSet.of(TABLE_REF)); verifyNoMoreInteractions(delegate); } @@ -210,7 +207,7 @@ public void get() throws Exception { Map result = kvs.get(TABLE_REF, cells); assertThat(result, equalTo(expectedResult)); - checkSpan("atlasdb-kvs.get(test.testTable, 1 cells)"); + checkSpan("atlasdb-kvs.get({table}, 1 cells)"); verify(delegate).get(TABLE_REF, cells); verifyNoMoreInteractions(delegate); } @@ -229,7 +226,7 @@ public void getAllTimestamps() throws Exception { Set cells = ImmutableSet.of(CELL); kvs.getAllTimestamps(TABLE_REF, cells, 1L); - checkSpan("atlasdb-kvs.getAllTimestamps(test.testTable, 1 keys, ts 1)"); + checkSpan("atlasdb-kvs.getAllTimestamps({table}, 1 keys, ts 1)"); verify(delegate).getAllTimestamps(TABLE_REF, cells, 1L); verifyNoMoreInteractions(delegate); } @@ -252,7 +249,7 @@ public void getFirstBatchForRanges() throws Exception { TABLE_REF, RANGE_REQUESTS, TIMESTAMP); assertThat(result, equalTo(expectedResult)); - checkSpan("atlasdb-kvs.getFirstBatchForRanges(test.testTable, 1 ranges, ts 1)"); + checkSpan("atlasdb-kvs.getFirstBatchForRanges({table}, 1 ranges, ts 1)"); verify(delegate).getFirstBatchForRanges(TABLE_REF, RANGE_REQUESTS, TIMESTAMP); verifyNoMoreInteractions(delegate); } @@ -265,7 +262,7 @@ public void getLatestTimestamps() throws Exception { Map result = kvs.getLatestTimestamps(TABLE_REF, cells); assertThat(result.entrySet(), hasSize(1)); - checkSpan("atlasdb-kvs.getLatestTimestamps(test.testTable, 1 cells)"); + checkSpan("atlasdb-kvs.getLatestTimestamps({table}, 1 cells)"); verify(delegate).getLatestTimestamps(TABLE_REF, cells); verifyNoMoreInteractions(delegate); } @@ -277,7 +274,7 @@ public void getMetadataForTable() throws Exception { byte[] result = kvs.getMetadataForTable(TABLE_REF); assertThat(result, equalTo(METADATA_BYTES)); - checkSpan("atlasdb-kvs.getMetadataForTable(test.testTable)"); + checkSpan("atlasdb-kvs.getMetadataForTable({table})"); verify(delegate).getMetadataForTable(TABLE_REF); verifyNoMoreInteractions(delegate); } @@ -295,34 +292,6 @@ public void getMetadataForTables() throws Exception { verifyNoMoreInteractions(delegate); } - @Test - public void getRange() throws Exception { - ClosableIterator> expectedResult = ClosableIterators.wrap(ImmutableList.of( - RowResult.of(CELL, VALUE)).iterator()); - when(delegate.getRange(TABLE_REF, RANGE_REQUEST, TIMESTAMP)).thenReturn(expectedResult); - try (ClosableIterator> result = kvs.getRange(TABLE_REF, RANGE_REQUEST, TIMESTAMP)) { - assertThat(result, equalTo(expectedResult)); - checkSpan("atlasdb-kvs.getRange(test.testTable, ts 1)"); - verify(delegate).getRange(TABLE_REF, RANGE_REQUEST, TIMESTAMP); - verifyNoMoreInteractions(delegate); - } - } - - @Test - public void getRangeOfTimestamps() throws Exception { - Set longs = ImmutableSet.of(TIMESTAMP); - ClosableIterator>> expectedResult = ClosableIterators.wrap(ImmutableList.of( - RowResult.of(CELL, longs)).iterator()); - when(delegate.getRangeOfTimestamps(TABLE_REF, RANGE_REQUEST, TIMESTAMP)).thenReturn(expectedResult); - try (ClosableIterator>> result = kvs.getRangeOfTimestamps( - TABLE_REF, RANGE_REQUEST, TIMESTAMP)) { - assertThat(result, equalTo(expectedResult)); - checkSpan("atlasdb-kvs.getRangeOfTimestamps(test.testTable, ts 1)"); - verify(delegate).getRangeOfTimestamps(TABLE_REF, RANGE_REQUEST, TIMESTAMP); - verifyNoMoreInteractions(delegate); - } - } - @Test public void getRows() throws Exception { ImmutableList rows = ImmutableList.of(ROW_NAME); @@ -332,7 +301,7 @@ public void getRows() throws Exception { Map result = kvs.getRows(TABLE_REF, rows, ColumnSelection.all(), TIMESTAMP); assertThat(result, equalTo(expectedResult)); - checkSpan("atlasdb-kvs.getRows(test.testTable, 1 rows, ts 1)"); + checkSpan("atlasdb-kvs.getRows({table}, 1 rows, ts 1)"); verify(delegate).getRows(TABLE_REF, rows, ColumnSelection.all(), TIMESTAMP); verifyNoMoreInteractions(delegate); } @@ -348,27 +317,11 @@ public void getRowsColumnRangeBatch() throws Exception { Map result = kvs.getRowsColumnRange(TABLE_REF, rows, range, TIMESTAMP); assertThat(result, equalTo(expectedResult)); - checkSpan("atlasdb-kvs.getRowsColumnRange(test.testTable, 1 rows, ts 1)"); + checkSpan("atlasdb-kvs.getRowsColumnRange({table}, 1 rows, ts 1)"); verify(delegate).getRowsColumnRange(TABLE_REF, rows, range, TIMESTAMP); verifyNoMoreInteractions(delegate); } - @Test - public void getRowsColumnRange() throws Exception { - RowColumnRangeIterator expectedResult = mock(RowColumnRangeIterator.class); - List rows = ImmutableList.of(ROW_NAME); - ColumnRangeSelection range = new ColumnRangeSelection(COL_NAME, COL_NAME); - int cellBatchHint = 2; - when(delegate.getRowsColumnRange(TABLE_REF, rows, range, cellBatchHint, TIMESTAMP)).thenReturn(expectedResult); - - RowColumnRangeIterator result = kvs.getRowsColumnRange(TABLE_REF, rows, range, cellBatchHint, TIMESTAMP); - - assertThat(result, equalTo(expectedResult)); - checkSpan("atlasdb-kvs.getRowsColumnRange(test.testTable, 1 rows, 2 hint, ts 1)"); - verify(delegate).getRowsColumnRange(TABLE_REF, rows, range, cellBatchHint, TIMESTAMP); - verifyNoMoreInteractions(delegate); - } - @Test public void multiPut() throws Exception { Map> values = ImmutableMap.of(TABLE_REF, ImmutableMap.of(CELL, VALUE_BYTES)); @@ -384,7 +337,7 @@ public void put() throws Exception { Map values = ImmutableMap.of(CELL, VALUE_BYTES); kvs.put(TABLE_REF, values, TIMESTAMP); - checkSpan("atlasdb-kvs.put(test.testTable, 1 values, ts 1)"); + checkSpan("atlasdb-kvs.put({table}, 1 values, ts 1)"); verify(delegate).put(TABLE_REF, values, TIMESTAMP); verifyNoMoreInteractions(delegate); } @@ -393,7 +346,7 @@ public void put() throws Exception { public void putMetadataForTable() throws Exception { kvs.putMetadataForTable(TABLE_REF, METADATA_BYTES); - checkSpan("atlasdb-kvs.putMetadataForTable(test.testTable, 8 bytes)"); + checkSpan("atlasdb-kvs.putMetadataForTable({table}, 8 bytes)"); verify(delegate).putMetadataForTable(TABLE_REF, METADATA_BYTES); verifyNoMoreInteractions(delegate); } @@ -402,7 +355,7 @@ public void putMetadataForTable() throws Exception { public void putMetadataForTables() throws Exception { kvs.putMetadataForTables(ImmutableMap.of(TABLE_REF, METADATA_BYTES)); - checkSpan("atlasdb-kvs.putMetadataForTables([test.testTable])"); + checkSpan("atlasdb-kvs.putMetadataForTables([{table}])"); verify(delegate).putMetadataForTables(ImmutableMap.of(TABLE_REF, METADATA_BYTES)); verifyNoMoreInteractions(delegate); } @@ -412,7 +365,7 @@ public void putUnlessExists() throws Exception { Map values = ImmutableMap.of(CELL, VALUE_BYTES); kvs.putUnlessExists(TABLE_REF, values); - checkSpan("atlasdb-kvs.putUnlessExists(test.testTable, 1 values)"); + checkSpan("atlasdb-kvs.putUnlessExists({table}, 1 values)"); verify(delegate).putUnlessExists(TABLE_REF, values); verifyNoMoreInteractions(delegate); } @@ -422,7 +375,7 @@ public void putWithTimestamps() throws Exception { Multimap values = ImmutableMultimap.of(CELL, VALUE); kvs.putWithTimestamps(TABLE_REF, values); - checkSpan("atlasdb-kvs.putWithTimestamps(test.testTable, 1 values)"); + checkSpan("atlasdb-kvs.putWithTimestamps({table}, 1 values)"); verify(delegate).putWithTimestamps(TABLE_REF, values); verifyNoMoreInteractions(delegate); } @@ -431,7 +384,7 @@ public void putWithTimestamps() throws Exception { public void truncateTable() throws Exception { kvs.truncateTable(TABLE_REF); - checkSpan("atlasdb-kvs.truncateTable(test.testTable)"); + checkSpan("atlasdb-kvs.truncateTable({table})"); verify(delegate).truncateTable(TABLE_REF); verifyNoMoreInteractions(delegate); } @@ -440,7 +393,7 @@ public void truncateTable() throws Exception { public void truncateTables() throws Exception { kvs.truncateTables(ImmutableSet.of(TABLE_REF)); - checkSpan("atlasdb-kvs.truncateTables([test.testTable])"); + checkSpan("atlasdb-kvs.truncateTables([{table}])"); verify(delegate).truncateTables(ImmutableSet.of(TABLE_REF)); verifyNoMoreInteractions(delegate); } diff --git a/atlasdb-client/src/test/java/com/palantir/atlasdb/logging/LoggingArgsTest.java b/atlasdb-client/src/test/java/com/palantir/atlasdb/logging/LoggingArgsTest.java index 465787822f8..ac81b57a79f 100644 --- a/atlasdb-client/src/test/java/com/palantir/atlasdb/logging/LoggingArgsTest.java +++ b/atlasdb-client/src/test/java/com/palantir/atlasdb/logging/LoggingArgsTest.java @@ -234,4 +234,25 @@ public void returnsUnsafeBatchBatchColumnRangeEvenWhenContainsSafeColumns() { assertThat(LoggingArgs.batchColumnRangeSelection(SAFE_TABLE_REFERENCE, MIXED_BATCH_COLUMN_RANGE)) .isInstanceOf(UnsafeArg.class); } + + @Test + public void returnsSafeTableWhenTableIsSafe() { + assertThat(LoggingArgs.safeTableOrPlaceholder(SAFE_TABLE_REFERENCE)).isEqualTo(SAFE_TABLE_REFERENCE); + } + + @Test + public void returnsPlaceholderWhenTableIsUnsafe() { + assertThat(LoggingArgs.safeTableOrPlaceholder(UNSAFE_TABLE_REFERENCE)) + .isEqualTo(LoggingArgs.PLACEHOLDER_TABLE_REFERENCE); + } + + @Test + public void returnsTablesAndPlaceholderWhenTablesAreSafeAndUnsafe() { + List tables = ImmutableList.of(SAFE_TABLE_REFERENCE, UNSAFE_TABLE_REFERENCE); + List returnedList = Lists.newArrayList(LoggingArgs.safeTablesOrPlaceholder(tables)); + List expectedList = Lists.newArrayList(SAFE_TABLE_REFERENCE, + LoggingArgs.PLACEHOLDER_TABLE_REFERENCE); + + assertThat(returnedList).containsOnly(expectedList.toArray(new TableReference[expectedList.size()])); + } } diff --git a/docs/source/release_notes/release-notes.rst b/docs/source/release_notes/release-notes.rst index 58cd8fd2b0d..4be3f47d3e8 100644 --- a/docs/source/release_notes/release-notes.rst +++ b/docs/source/release_notes/release-notes.rst @@ -124,6 +124,10 @@ v0.65.1 - AtlasDB tables will now be logged as ``ns.tablename`` instead of ``map[namespace:map[name:ns] tablename:tablename]``. (`Pull Request `__) + * - |fixed| + - TracingKVS now has spans with safe names. + (`Pull Request `__) + .. <<<<------------------------------------------------------------------------------------------------------------->>>> =======