Skip to content

Commit b3a5068

Browse files
dougqhclaude
andcommitted
Add tag-id keyed removal to TagMap and TagMap.Ledger
- Move the lazy tagId->name resolution into the base EntryChange.tag() (final) and drop Entry's override, so EntryRemoval resolves its name from a tagId too. - EntryChange.newRemoval(long) / EntryRemoval(long) carry a tag id. - TagMap.remove(long)/getAndRemove(long): OptimizedTagMap clears the slot by id (knownRemove) then falls back to the resolved-name bucket lookup; LegacyTagMap resolves the name and delegates. - Ledger.remove(long) records an id-keyed removal; fill() replays id removals via map.remove(long) (slot-aware, no name round-trip), string removals by name. Tests: unit coverage for remove/getAndRemove by id (slot clear, prior value, string-set-then-remove-by-id, ledger remove-by-id) plus a fuzz removeById action woven into the random mix (exercises slot-clear + collided-slot reclaim); ~48k seeded sequences clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 93263f1 commit b3a5068

3 files changed

Lines changed: 151 additions & 12 deletions

File tree

internal-api/src/main/java/datadog/trace/api/TagMap.java

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,12 @@ static Ledger ledger(int size) {
253253
*/
254254
Entry getAndRemove(String tag);
255255

256+
/** Tag-id keyed removal (no prior value). See {@link #remove(String)}. */
257+
boolean remove(long tagId);
258+
259+
/** Tag-id keyed removal returning the prior Entry. See {@link #getAndRemove(String)}. */
260+
Entry getAndRemove(long tagId);
261+
256262
/** Returns a mutable copy of this TagMap */
257263
TagMap copy();
258264

@@ -310,6 +316,10 @@ public static final EntryRemoval newRemoval(String tag) {
310316
return new EntryRemoval(tag);
311317
}
312318

319+
public static final EntryRemoval newRemoval(long tagId) {
320+
return new EntryRemoval(tagId);
321+
}
322+
313323
// tagId encoding: bits 63-48 = globalSerial (0 for unknown tags), bits 47-32 = fieldPos,
314324
// bits 31-0 = nameHash (_hash(tagName)). String-constructed entries have upper 32 bits zero
315325
// with the hash lazily populated on first hash(). tagId-constructed entries have all bits set
@@ -331,8 +341,15 @@ public static final EntryRemoval newRemoval(String tag) {
331341
this.tag = null; // resolved lazily via tag()
332342
}
333343

334-
public String tag() {
335-
return this.tag;
344+
// For tagId-constructed changes (entries and removals) the name is resolved lazily from the
345+
// tagId via KnownTags and cached. Benign race: KnownTags.nameOf returns the same interned
346+
// constant, so re-resolution is harmless.
347+
public final String tag() {
348+
String name = this.tag;
349+
if (name != null) return name;
350+
name = KnownTags.nameOf(this.tagId);
351+
if (name != null) this.tag = name;
352+
return name;
336353
}
337354

338355
public final boolean matches(String tag) {
@@ -354,6 +371,10 @@ final class EntryRemoval extends EntryChange {
354371
super(tag);
355372
}
356373

374+
EntryRemoval(long tagId) {
375+
super(tagId);
376+
}
377+
357378
@Override
358379
public boolean isRemoval() {
359380
return true;
@@ -583,15 +604,6 @@ int hash() {
583604
return hash;
584605
}
585606

586-
@Override
587-
public String tag() {
588-
String name = this.tag;
589-
if (name != null) return name;
590-
name = KnownTags.nameOf(this.tagId);
591-
if (name != null) this.tag = name;
592-
return name;
593-
}
594-
595607
@Override
596608
public Entry entry() {
597609
return this;
@@ -1160,6 +1172,10 @@ public Ledger remove(String tag) {
11601172
return this.recordRemoval(EntryChange.newRemoval(tag));
11611173
}
11621174

1175+
public Ledger remove(long tagId) {
1176+
return this.recordRemoval(EntryChange.newRemoval(tagId));
1177+
}
1178+
11631179
private Ledger recordEntry(Entry entry) {
11641180
this.recordChange(entry);
11651181
return this;
@@ -1243,7 +1259,12 @@ void fill(TagMap map) {
12431259
EntryChange change = entryChanges[i];
12441260

12451261
if (change.isRemoval()) {
1246-
map.remove(change.tag());
1262+
// route tag-id removals by id (slot-aware, no name round-trip); string removals by name
1263+
if (KnownTags.globalSerial(change.tagId) != 0) {
1264+
map.remove(change.tagId);
1265+
} else {
1266+
map.remove(change.tag());
1267+
}
12471268
} else {
12481269
map.set((Entry) change);
12491270
}
@@ -2133,6 +2154,24 @@ public boolean remove(String tag) {
21332154
return (this.getAndRemove(tag) != null);
21342155
}
21352156

2157+
@Override
2158+
public boolean remove(long tagId) {
2159+
return (this.getAndRemove(tagId) != null);
2160+
}
2161+
2162+
@Override
2163+
public Entry getAndRemove(long tagId) {
2164+
this.checkWriteAccess();
2165+
2166+
// known tags live in their slot - clear there first (by id, no name needed)
2167+
Entry slotEntry = this.knownRemove(tagId);
2168+
if (slotEntry != null) return slotEntry;
2169+
2170+
// otherwise it may have collided into the buckets - look up by resolved name
2171+
String name = KnownTags.nameOf(tagId);
2172+
return name == null ? null : this.removeFromBuckets(name, KnownTags.nameHash(tagId));
2173+
}
2174+
21362175
@Override
21372176
public Entry getAndRemove(String tag) {
21382177
this.checkWriteAccess();
@@ -3426,6 +3465,19 @@ public TagMap.Entry getAndRemove(String tag) {
34263465
return prior == null ? null : TagMap.Entry.newAnyEntry(tag, prior);
34273466
}
34283467

3468+
// Tag-id keyed removals: LegacyTagMap is name-keyed, so resolve the name and delegate.
3469+
@Override
3470+
public boolean remove(long tagId) {
3471+
String name = KnownTags.nameOf(tagId);
3472+
return name != null && this.remove(name);
3473+
}
3474+
3475+
@Override
3476+
public TagMap.Entry getAndRemove(long tagId) {
3477+
String name = KnownTags.nameOf(tagId);
3478+
return name == null ? null : this.getAndRemove(name);
3479+
}
3480+
34293481
@Override
34303482
public Object getObject(String tag) {
34313483
return this.get(tag);

internal-api/src/test/java/datadog/trace/api/TagMapFuzzTest.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,6 +1062,7 @@ public static final MapAction randomAction() {
10621062
return randomChoice(
10631063
() -> remove(randomKey()),
10641064
() -> removeLight(randomKey()),
1065+
() -> removeById(randomKey()),
10651066
() -> getAndRemove(randomKey()));
10661067

10671068
default:
@@ -1117,6 +1118,10 @@ public static final MapAction removeLight(String key) {
11171118
return new RemoveLight(key);
11181119
}
11191120

1121+
public static final MapAction removeById(String key) {
1122+
return new RemoveById(key);
1123+
}
1124+
11201125
public static final MapAction getAndRemove(String key) {
11211126
return new GetAndRemove(key);
11221127
}
@@ -1679,6 +1684,39 @@ public String toString() {
16791684
}
16801685
}
16811686

1687+
static final class RemoveById extends ReturningAction<Object, Boolean> {
1688+
final String key;
1689+
1690+
RemoveById(String key) {
1691+
this.key = key;
1692+
}
1693+
1694+
@Override
1695+
protected Boolean _applyToTestMap(TagMap testMap) {
1696+
return testMap.remove(tagIdOf(this.key));
1697+
}
1698+
1699+
@Override
1700+
protected Object _applyToExpectedMap(Map<String, Object> expectedMap) {
1701+
return expectedMap.remove(this.key);
1702+
}
1703+
1704+
@Override
1705+
protected void _verifyResults(Object expected, Boolean actual) {
1706+
assertEquals((expected != null), actual);
1707+
}
1708+
1709+
@Override
1710+
public void verifyTestMap(TagMap testMap) {
1711+
assertFalse(testMap.containsKey(this.key));
1712+
}
1713+
1714+
@Override
1715+
public String toString() {
1716+
return String.format("removeById(%s)", literal(this.key));
1717+
}
1718+
}
1719+
16821720
static final class GetAndRemove extends ReturningAction<Object, TagMap.Entry> {
16831721
final String key;
16841722

internal-api/src/test/java/datadog/trace/api/TagMapTagIdTest.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package datadog.trace.api;
22

33
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertFalse;
45
import static org.junit.jupiter.api.Assertions.assertNotNull;
56
import static org.junit.jupiter.api.Assertions.assertNull;
67
import static org.junit.jupiter.api.Assertions.assertSame;
@@ -172,4 +173,52 @@ public void ledger_mixedIdAndName() {
172173
assertEquals("redis", map.get(DB_SYSTEM));
173174
assertEquals(2, map.size());
174175
}
176+
177+
@Test
178+
public void removeById_clearsAndReportsSize() {
179+
TagMap map = TagMap.create();
180+
map.set(HTTP_METHOD_ID, "GET");
181+
assertEquals(1, map.size());
182+
183+
assertTrue(map.remove(HTTP_METHOD_ID));
184+
assertNull(map.getEntry(HTTP_METHOD_ID));
185+
assertNull(map.getEntry(HTTP_METHOD));
186+
assertEquals(0, map.size());
187+
assertFalse(map.remove(HTTP_METHOD_ID)); // already gone
188+
}
189+
190+
@Test
191+
public void getAndRemoveById_returnsPrior() {
192+
TagMap map = TagMap.create();
193+
map.set(HTTP_STATUS_ID, 404);
194+
195+
Entry prior = map.getAndRemove(HTTP_STATUS_ID);
196+
assertNotNull(prior);
197+
assertEquals(404, prior.intValue());
198+
assertNull(map.getEntry(HTTP_STATUS_ID));
199+
}
200+
201+
@Test
202+
public void removeById_removesStringSetTag() {
203+
// set by name, removed by id (id resolves to the same tag, slot or bucket)
204+
TagMap map = TagMap.create();
205+
map.set(DB_SYSTEM, "postgresql");
206+
207+
assertTrue(map.remove(DB_SYSTEM_ID));
208+
assertNull(map.get(DB_SYSTEM));
209+
}
210+
211+
@Test
212+
public void ledger_removeById() {
213+
TagMap map =
214+
TagMap.ledger()
215+
.set(HTTP_METHOD_ID, "GET")
216+
.set(DB_SYSTEM_ID, "mysql")
217+
.remove(DB_SYSTEM_ID)
218+
.build();
219+
220+
assertEquals("GET", map.get(HTTP_METHOD));
221+
assertNull(map.get(DB_SYSTEM));
222+
assertEquals(1, map.size());
223+
}
175224
}

0 commit comments

Comments
 (0)