Skip to content

Commit 0388491

Browse files
committed
NCBC-4138: Several Tracing issues
Motivation ========== Spans were missing modern OTel tags when convention was set to Both or Modern, cluster name changes weren't propagated, and KV dispatch spans misattributed the resolved IP as server.address. Modification ============ - Changed Convention from init-only to re-resolvable; Cluster.cs calls ResolveAndSetConvention() after assigning ClusterContext. - RequestSpanWrapper.ChildSpan now wraps child spans so attributes route through SemanticConventionEmitter. - Added value mapping to SemanticConventionEmitter ("IP.TCP" → "tcp"). - Rewrote UpdateClusterLabelsIfNecessary as MergeClusterLabels: the old method copied stale labels onto incoming configs, preventing server-provided name/uuid changes from propagating. The new method prefers incoming server values and writes the merged result to both configs, which is necessary because GlobalConfig is not always replaced (the early return in ConfigUpdatedAsync when there are no structural changes would otherwise discard the updated labels). - Threaded logical hostname through MultiplexingConnection/SslConnection so AddTags writes logical hostname to net.peer.name and resolved IP to new network.peer.address/network.peer.port attributes. Note: net.peer.name on KV dispatch spans now emits the logical hostname instead of the resolved IP in all convention modes. This aligns with the spec but changes existing behavior for consumers parsing this value. Results ======= All existing and new unit tests pass. Spans now correctly emit modern convention tags for both parent and child spans. Observability tests all pass in FIT. Change-Id: Ifc65f01543129803e233879f15cda041cc0c53ce Reviewed-on: https://review.couchbase.org/c/couchbase-net-client/+/242461 Reviewed-by: Jeffry Morris <jeffrymorris@gmail.com> Tested-by: Build Bot <build@couchbase.com>
1 parent f7425ee commit 0388491

13 files changed

Lines changed: 330 additions & 24 deletions

File tree

src/Couchbase/Cluster.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ internal Cluster(ClusterOptions clusterOptions)
112112
if (_tracer is RequestTracerWrapper wrapper)
113113
{
114114
wrapper.ClusterContext = _context;
115+
wrapper.ResolveAndSetConvention();
115116
}
116117
_retryStrategy = _context.ServiceProvider.GetRequiredService<IRetryStrategy>();
117118

src/Couchbase/Core/ClusterContext.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -932,8 +932,12 @@ public async Task ConfigUpdatedAsync(BucketConfig globalConfig)
932932
}
933933
#endif
934934

935-
// Change ClusterLabels if new ones have been received (can come from either Global or Bucket configs)
936-
GlobalConfig?.UpdateClusterLabelsIfNecessary(globalConfig);
935+
// Merge ClusterLabels: prefer incoming values (latest from server), carry forward current values as fallback.
936+
// Both configs are updated so labels survive even if GlobalConfig is replaced.
937+
if (GlobalConfig is not null)
938+
{
939+
globalConfig.MergeClusterLabels(GlobalConfig);
940+
}
937941

938942
if (globalConfig.Name == Name)
939943
{

src/Couchbase/Core/Configuration/Server/BucketConfigExtensions.cs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,19 @@ public static bool HasClusterNodesChanged(this BucketConfig config, BucketConfig
3636
return !(config.NodesExt.AreEqual(other.NodesExt) && config.Nodes.AreEqual(other.Nodes));
3737
}
3838

39-
public static void UpdateClusterLabelsIfNecessary(this BucketConfig newConfig, BucketConfig oldConfig)
39+
public static void MergeClusterLabels(this BucketConfig incomingConfig, BucketConfig currentConfig)
4040
{
41-
if (newConfig.ClusterLabels.Equals(oldConfig.ClusterLabels!)) return;
42-
if (newConfig.ClusterLabels.ClusterName is not null)
43-
{
44-
oldConfig.ClusterLabels.ClusterName = newConfig.ClusterLabels.ClusterName;
45-
}
41+
if (incomingConfig.ClusterLabels.Equals(currentConfig.ClusterLabels)) return;
4642

47-
if (newConfig.ClusterLabels.ClusterUuid is not null)
48-
{
49-
oldConfig.ClusterLabels.ClusterUuid = newConfig.ClusterLabels.ClusterUuid;
50-
}
43+
// Prefer incoming values (latest from server), fall back to current values
44+
var name = incomingConfig.ClusterLabels.ClusterName ?? currentConfig.ClusterLabels.ClusterName;
45+
var uuid = incomingConfig.ClusterLabels.ClusterUuid ?? currentConfig.ClusterLabels.ClusterUuid;
46+
47+
// Apply to both so labels are correct regardless of which config is used going forward
48+
incomingConfig.ClusterLabels.ClusterName = name;
49+
incomingConfig.ClusterLabels.ClusterUuid = uuid;
50+
currentConfig.ClusterLabels.ClusterName = name;
51+
currentConfig.ClusterLabels.ClusterUuid = uuid;
5152
}
5253
public static bool HasConfigChanges(this BucketConfig newConfig, BucketConfig? oldConfig, string bucketName)
5354
{

src/Couchbase/Core/Diagnostics/SemanticConventionEmitter.cs

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,37 @@ internal static class SemanticConventionEmitter
4444
};
4545
#endif
4646

47+
// Value mapping: legacy attribute value -> modern attribute value.
48+
// Applied only when emitting in Modern or Both mode for string-typed attributes.
49+
#if NET8_0_OR_GREATER
50+
private static readonly FrozenDictionary<string, string> LegacyToModernValues =
51+
new Dictionary<string, string>(StringComparer.Ordinal)
52+
{
53+
#else
54+
private static readonly Dictionary<string, string> LegacyToModernValues =
55+
new Dictionary<string, string>(StringComparer.Ordinal)
56+
{
57+
#endif
58+
["IP.TCP"] = "tcp",
59+
#if NET8_0_OR_GREATER
60+
}.ToFrozenDictionary(StringComparer.Ordinal);
61+
#else
62+
};
63+
#endif
64+
65+
/// <summary>
66+
/// Maps a legacy string value to its modern equivalent, if one exists.
67+
/// </summary>
68+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
69+
private static T MapValue<T>(T value)
70+
{
71+
if (value is string strValue && LegacyToModernValues.TryGetValue(strValue, out var modernValue))
72+
{
73+
return (T)(object)modernValue;
74+
}
75+
return value;
76+
}
77+
4778
/// <summary>
4879
/// Emits an attribute using the appropriate semantic convention key(s).
4980
/// The <paramref name="state"/> parameter is forwarded to the callback, allowing
@@ -76,13 +107,13 @@ internal static void EmitAttribute<TState, T>(
76107
{
77108
case ObservabilitySemanticConvention.Modern:
78109
if (modernKey.Length == 0) return;
79-
setAttribute(state, modernKey, value);
110+
setAttribute(state, modernKey, MapValue(value));
80111
return;
81112

82113
case ObservabilitySemanticConvention.Both:
83114
setAttribute(state, key, value);
84115
if (modernKey.Length == 0) return;
85-
setAttribute(state, modernKey, value);
116+
setAttribute(state, modernKey, MapValue(value));
86117
return;
87118

88119
default:

src/Couchbase/Core/Diagnostics/Tracing/InnerRequestSpans.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,16 @@ internal static class Attributes
129129
/// The operation timeout in milliseconds
130130
/// </summary>
131131
public const string TimeoutMilliseconds = "timeout_ms";
132+
133+
/// <summary>
134+
/// The resolved IP address of the remote peer (modern OTel convention).
135+
/// </summary>
136+
public const string NetworkPeerAddress = "network.peer.address";
137+
138+
/// <summary>
139+
/// The port of the remote peer (modern OTel convention).
140+
/// </summary>
141+
public const string NetworkPeerPort = "network.peer.port";
132142
}
133143
}
134144
}

src/Couchbase/Core/Diagnostics/Tracing/RequestSpanWrapper.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public IRequestSpan Parent
5656
}
5757
public IRequestSpan ChildSpan(string name)
5858
{
59-
return innerSpan.ChildSpan(name);
59+
return new RequestSpanWrapper(innerSpan.ChildSpan(name), clusterLabels, convention);
6060
}
6161

6262
public bool CanWrite => innerSpan.CanWrite;

src/Couchbase/Core/Diagnostics/Tracing/RequestTracerWrapper.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,15 @@ internal sealed class RequestTracerWrapper(IRequestTracer innerTracer, ClusterCo
66
internal ClusterContext ClusterContext = clusterContext;
77
internal ClusterLabels ClusterLabels => ClusterContext?.GlobalConfig?.ClusterLabels;
88

9-
internal ObservabilitySemanticConvention Convention { get; } = ResolveConvention(clusterContext);
9+
internal ObservabilitySemanticConvention Convention { get; private set; } = ResolveConvention(clusterContext);
10+
11+
/// <summary>
12+
/// Re-resolves the Convention from ClusterContext. Call once after ClusterContext is assigned.
13+
/// </summary>
14+
internal void ResolveAndSetConvention()
15+
{
16+
Convention = ResolveConvention(ClusterContext);
17+
}
1018

1119
private static ObservabilitySemanticConvention ResolveConvention(ClusterContext clusterContext)
1220
{

src/Couchbase/Core/IO/Connections/ConnectionFactory.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,10 @@ public async Task<IConnection> CreateAndConnectAsync(HostEndpointWithPort hostEn
126126
if (!isSecure) throw new AuthenticationException($"The SSL/TLS connection could not be authenticated on [{targetHost}].");
127127

128128
return new SslConnection(sslStream, _clusterOptions.Tuning.MaximumInFlightOperationsPerConnection,
129-
socket.LocalEndPoint!, socket.RemoteEndPoint!, _sslLogger, _multiplexLogger);
129+
socket.LocalEndPoint!, socket.RemoteEndPoint!, _sslLogger, _multiplexLogger, hostEndpoint.Host);
130130
}
131131

132-
return new MultiplexingConnection(socket, _clusterOptions.Tuning.MaximumInFlightOperationsPerConnection, _multiplexLogger);
132+
return new MultiplexingConnection(socket, _clusterOptions.Tuning.MaximumInFlightOperationsPerConnection, _multiplexLogger, hostEndpoint.Host);
133133
}
134134
}
135135
}

src/Couchbase/Core/IO/Connections/MultiplexingConnection.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,20 +42,23 @@ internal sealed class MultiplexingConnection : IConnection
4242
private readonly string _localHostString;
4343
private readonly string _remotePortString;
4444
private readonly string _localPortString;
45+
private readonly string _logicalRemoteHostname;
4546

4647
// Connection pooling normally prevents simultaneous writes, but there are cases where they may occur,
4748
// such as when running Diagnostics pings. We therefore need to prevent them ourselves, as the internal
4849
// implementation of socket writes may interleave large buffers written from different threads.
4950
private readonly SemaphoreSlim _writeMutex = new(1);
5051

51-
public MultiplexingConnection(Socket socket, int maximumInFlightOperations, ILogger<MultiplexingConnection> logger)
52+
public MultiplexingConnection(Socket socket, int maximumInFlightOperations,
53+
ILogger<MultiplexingConnection> logger, string? logicalRemoteHostname = null)
5254
: this(new NetworkStream(socket, true), maximumInFlightOperations,
53-
socket.LocalEndPoint!, socket.RemoteEndPoint!, logger)
55+
socket.LocalEndPoint!, socket.RemoteEndPoint!, logger, logicalRemoteHostname)
5456
{
5557
}
5658

5759
public MultiplexingConnection(Stream stream, int maximumInFlightOperations,
58-
EndPoint localEndPoint, EndPoint remoteEndPoint, ILogger<MultiplexingConnection> logger)
60+
EndPoint localEndPoint, EndPoint remoteEndPoint, ILogger<MultiplexingConnection> logger,
61+
string? logicalRemoteHostname = null)
5962
{
6063
_stream = stream ?? throw new ArgumentNullException(nameof(stream));
6164
LocalEndPoint = localEndPoint ?? throw new ArgumentNullException(nameof(localEndPoint));
@@ -71,6 +74,7 @@ public MultiplexingConnection(Stream stream, int maximumInFlightOperations,
7174
_localHostString = ((IPEndPoint) LocalEndPoint).Address.ToString() ?? DiagnosticsReportProvider.UnknownEndpointValue;
7275
_remotePortString = ((IPEndPoint) EndPoint).Port.ToStringInvariant();
7376
_localPortString = ((IPEndPoint) LocalEndPoint).Port.ToStringInvariant();
77+
_logicalRemoteHostname = logicalRemoteHostname ?? _remoteHostString;
7478

7579
EndpointState = EndpointState.Connecting;
7680

@@ -492,8 +496,10 @@ public void AddTags(IRequestSpan span)
492496
{
493497
span.SetAttribute(InnerRequestSpans.DispatchSpan.Attributes.LocalHostname, _localHostString);
494498
span.SetAttribute(InnerRequestSpans.DispatchSpan.Attributes.LocalPort, _localPortString);
495-
span.SetAttribute(InnerRequestSpans.DispatchSpan.Attributes.RemoteHostname, _remoteHostString);
499+
span.SetAttribute(InnerRequestSpans.DispatchSpan.Attributes.RemoteHostname, _logicalRemoteHostname);
496500
span.SetAttribute(InnerRequestSpans.DispatchSpan.Attributes.RemotePort, _remotePortString);
501+
span.SetAttribute(InnerRequestSpans.DispatchSpan.Attributes.NetworkPeerAddress, _remoteHostString);
502+
span.SetAttribute(InnerRequestSpans.DispatchSpan.Attributes.NetworkPeerPort, _remotePortString);
497503
span.SetAttribute(InnerRequestSpans.DispatchSpan.Attributes.LocalId, ContextId);
498504
}
499505
}

src/Couchbase/Core/IO/Connections/SslConnection.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@ internal sealed class SslConnection : IConnection
2020
private readonly MultiplexingConnection _multiplexingConnection;
2121

2222
public SslConnection(SslStream stream, int maximumInFlightOperations, EndPoint localEndPoint, EndPoint remoteEndPoint,
23-
ILogger<SslConnection> logger, ILogger<MultiplexingConnection> multiplexingLogger)
23+
ILogger<SslConnection> logger, ILogger<MultiplexingConnection> multiplexingLogger,
24+
string? logicalRemoteHostname = null)
2425
{
2526
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
2627

2728
_sslStream = stream ?? throw new ArgumentNullException(nameof(stream));
2829
_multiplexingConnection = new MultiplexingConnection(_sslStream, maximumInFlightOperations,
29-
localEndPoint, remoteEndPoint, multiplexingLogger);
30+
localEndPoint, remoteEndPoint, multiplexingLogger, logicalRemoteHostname);
3031
}
3132

3233
public string ContextId => _multiplexingConnection.ContextId;

0 commit comments

Comments
 (0)