-
Notifications
You must be signed in to change notification settings - Fork 293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Instrumentation.StackExchangeRedis] Metrics support #1982
Changes from all commits
45816c4
d343c6c
f3b1d3e
254211a
b1662bb
8d4befa
eedaf7c
59264cf
b0ae03b
2fa5346
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
using System.Diagnostics.Metrics; | ||
using System.Reflection; | ||
using OpenTelemetry.Internal; | ||
|
||
namespace OpenTelemetry.Instrumentation.StackExchangeRedis.Implementation; | ||
|
||
internal class RedisMetrics : IDisposable | ||
{ | ||
internal static readonly Assembly Assembly = typeof(StackExchangeRedisInstrumentation).Assembly; | ||
internal static readonly AssemblyName AssemblyName = Assembly.GetName(); | ||
internal static readonly string InstrumentationName = AssemblyName.Name!; | ||
internal static readonly string InstrumentationVersion = Assembly.GetPackageVersion(); | ||
Kielek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
private readonly Meter meter; | ||
|
||
public RedisMetrics() | ||
{ | ||
this.meter = new Meter(InstrumentationName, InstrumentationVersion); | ||
|
||
this.DurationHistogram = this.meter.CreateHistogram<double>( | ||
"db.client.operation.duration", | ||
unit: "s", | ||
description: "Duration of database client operations."); | ||
} | ||
|
||
public static RedisMetrics Instance { get; } = new RedisMetrics(); | ||
|
||
public Histogram<double> DurationHistogram { get; } | ||
|
||
public bool Enabled => this.DurationHistogram.Enabled; | ||
|
||
public void Dispose() | ||
{ | ||
this.meter.Dispose(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,15 +7,18 @@ | |
using System.Reflection.Emit; | ||
using OpenTelemetry.Trace; | ||
using StackExchange.Redis.Profiling; | ||
|
||
#if NET | ||
using System.Diagnostics.CodeAnalysis; | ||
using System.Runtime.CompilerServices; | ||
#endif | ||
|
||
#endif | ||
namespace OpenTelemetry.Instrumentation.StackExchangeRedis.Implementation; | ||
|
||
internal static class RedisProfilerEntryToActivityConverter | ||
internal static class RedisProfilerEntryInstrumenter | ||
{ | ||
private const int DefaultPort = 6379; | ||
|
||
private static readonly Lazy<Func<object, (string?, string?)>> MessageDataGetter = new(() => | ||
{ | ||
var profiledCommandType = Type.GetType("StackExchange.Redis.Profiling.ProfiledCommand, StackExchange.Redis", throwOnError: true)!; | ||
|
@@ -68,7 +71,11 @@ static bool GetCommandAndKey( | |
}); | ||
}); | ||
|
||
public static Activity? ProfilerCommandToActivity(Activity? parentActivity, IProfiledCommand command, StackExchangeRedisInstrumentationOptions options) | ||
public static Activity? ProfilerCommandInstrument( | ||
Activity? parentActivity, | ||
IProfiledCommand command, | ||
RedisMetrics metrics, | ||
StackExchangeRedisInstrumentationOptions options) | ||
{ | ||
var name = command.Command; // Example: SET; | ||
if (string.IsNullOrEmpty(name)) | ||
|
@@ -83,30 +90,38 @@ static bool GetCommandAndKey( | |
StackExchangeRedisConnectionInstrumentation.CreationTags, | ||
startTime: command.CommandCreated); | ||
|
||
if (activity == null) | ||
if (activity is null && metrics.Enabled is false) | ||
{ | ||
return null; | ||
} | ||
|
||
activity.SetEndTime(command.CommandCreated + command.ElapsedTime); | ||
activity?.SetEndTime(command.CommandCreated + command.ElapsedTime); | ||
var meterTags = metrics.Enabled ? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Isn't this sufficient? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the result of Here I would like to set a null object to be validated in every AddTag and so optimize the memory allocation |
||
new TagList(StackExchangeRedisConnectionInstrumentation.CreationTags) : | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. creationtags is no longer needed, as it is already covered via Meter Tags. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since you suggest removing the meter tags #1982 (comment), I will keep it |
||
default(IList<KeyValuePair<string, object?>>); | ||
|
||
if (activity.IsAllDataRequested) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is worth to keep this functionality. Checks should be cheaper than adding couple of tags. |
||
{ | ||
// see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md | ||
// see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md | ||
|
||
// Timing example: | ||
// command.CommandCreated; //2019-01-10 22:18:28Z | ||
|
||
// Timing example: | ||
// command.CommandCreated; //2019-01-10 22:18:28Z | ||
// command.CreationToEnqueued; // 00:00:32.4571995 | ||
// command.EnqueuedToSending; // 00:00:00.0352838 | ||
// command.SentToResponse; // 00:00:00.0060586 | ||
// command.ResponseToCompletion; // 00:00:00.0002601 | ||
|
||
// command.CreationToEnqueued; // 00:00:32.4571995 | ||
// command.EnqueuedToSending; // 00:00:00.0352838 | ||
// command.SentToResponse; // 00:00:00.0060586 | ||
// command.ResponseToCompletion; // 00:00:00.0002601 | ||
// Total: | ||
// command.ElapsedTime; // 00:00:32.4988020 | ||
|
||
// Total: | ||
// command.ElapsedTime; // 00:00:32.4988020 | ||
activity?.SetTag(SemanticConventions.AttributeDbRedisFlagsKeyName, command.Flags.ToString()); | ||
|
||
activity.SetTag(StackExchangeRedisConnectionInstrumentation.RedisFlagsKeyName, command.Flags.ToString()); | ||
if (command.Command != null) | ||
{ | ||
meterTags?.Add(SemanticConventions.AttributeDbOperationName, command.Command); | ||
} | ||
|
||
if (activity != null) | ||
{ | ||
if (options.SetVerboseDatabaseStatements) | ||
{ | ||
var (commandAndKey, script) = MessageDataGetter.Value.Invoke(command); | ||
|
@@ -130,31 +145,59 @@ static bool GetCommandAndKey( | |
// Example: "db.statement": SET; | ||
activity.SetTag(SemanticConventions.AttributeDbStatement, command.Command); | ||
} | ||
} | ||
|
||
if (command.EndPoint != null) | ||
if (command.EndPoint != null) | ||
{ | ||
if (command.EndPoint is IPEndPoint ipEndPoint) | ||
{ | ||
if (command.EndPoint is IPEndPoint ipEndPoint) | ||
{ | ||
activity.SetTag(SemanticConventions.AttributeNetPeerIp, ipEndPoint.Address.ToString()); | ||
activity.SetTag(SemanticConventions.AttributeNetPeerPort, ipEndPoint.Port); | ||
} | ||
else if (command.EndPoint is DnsEndPoint dnsEndPoint) | ||
var ip = ipEndPoint.Address.ToString(); | ||
var port = ipEndPoint.Port; | ||
|
||
activity?.SetTag(SemanticConventions.AttributeNetPeerIp, ip); | ||
activity?.SetTag(SemanticConventions.AttributeNetPeerPort, port); | ||
|
||
meterTags?.Add(SemanticConventions.AttributeServerAddress, ip); | ||
meterTags?.Add(SemanticConventions.AttributeNetworkPeerAddress, ip); | ||
if (port != DefaultPort) | ||
{ | ||
activity.SetTag(SemanticConventions.AttributeNetPeerName, dnsEndPoint.Host); | ||
activity.SetTag(SemanticConventions.AttributeNetPeerPort, dnsEndPoint.Port); | ||
meterTags?.Add(SemanticConventions.AttributeServerPort, port); | ||
meterTags?.Add(SemanticConventions.AttributeNetworkPeerPort, port); | ||
} | ||
else | ||
} | ||
else if (command.EndPoint is DnsEndPoint dnsEndPoint) | ||
{ | ||
var host = dnsEndPoint.Host; | ||
var port = dnsEndPoint.Port; | ||
|
||
activity?.SetTag(SemanticConventions.AttributeNetPeerName, host); | ||
activity?.SetTag(SemanticConventions.AttributeNetPeerPort, port); | ||
Comment on lines
+173
to
+174
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Candidate for separate PR - update semantic convention to 1.27.0. At least for network tags. Note. |
||
|
||
meterTags?.Add(SemanticConventions.AttributeServerAddress, host); | ||
if (port != DefaultPort) | ||
{ | ||
activity.SetTag(SemanticConventions.AttributePeerService, command.EndPoint.ToString()); | ||
meterTags?.Add(SemanticConventions.AttributeServerPort, port); | ||
} | ||
} | ||
else | ||
{ | ||
var service = command.EndPoint.ToString(); | ||
|
||
activity?.SetTag(SemanticConventions.AttributePeerService, service); | ||
meterTags?.Add(SemanticConventions.AttributeServerAddress, service); | ||
} | ||
} | ||
|
||
activity.SetTag(StackExchangeRedisConnectionInstrumentation.RedisDatabaseIndexKeyName, command.Db); | ||
var db = command.Db; | ||
activity?.SetTag(SemanticConventions.AttributeDbRedisDatabaseIndex, db); | ||
meterTags?.Add(SemanticConventions.AttributeDbNamespace, db); | ||
|
||
// TODO: deal with the re-transmission | ||
// command.RetransmissionOf; | ||
// command.RetransmissionReason; | ||
// TODO: deal with the re-transmission | ||
// command.RetransmissionOf; | ||
// command.RetransmissionReason; | ||
|
||
if (activity?.IsAllDataRequested ?? false) | ||
{ | ||
var enqueued = command.CommandCreated.Add(command.CreationToEnqueued); | ||
var send = enqueued.Add(command.EnqueuedToSending); | ||
var response = send.Add(command.SentToResponse); | ||
|
@@ -169,16 +212,25 @@ static bool GetCommandAndKey( | |
options.Enrich?.Invoke(activity, command); | ||
} | ||
|
||
activity.Stop(); | ||
if (metrics.Enabled && meterTags is TagList meterTagList) | ||
{ | ||
metrics.DurationHistogram.Record(command.ElapsedTime.TotalSeconds, meterTagList); | ||
} | ||
|
||
activity?.Stop(); | ||
|
||
return activity; | ||
} | ||
|
||
public static void DrainSession(Activity? parentActivity, IEnumerable<IProfiledCommand> sessionCommands, StackExchangeRedisInstrumentationOptions options) | ||
public static void DrainSession( | ||
Activity? parentActivity, | ||
IEnumerable<IProfiledCommand> sessionCommands, | ||
RedisMetrics redisMetrics, | ||
StackExchangeRedisInstrumentationOptions options) | ||
{ | ||
foreach (var command in sessionCommands) | ||
{ | ||
ProfilerCommandToActivity(parentActivity, command, options); | ||
ProfilerCommandInstrument(parentActivity, command, redisMetrics, options); | ||
} | ||
} | ||
|
||
|
@@ -224,4 +276,9 @@ public static void DrainSession(Activity? parentActivity, IEnumerable<IProfiledC | |
|
||
return null; | ||
} | ||
|
||
private static void Add(this IList<KeyValuePair<string, object?>> tags, string ket, object? value) | ||
{ | ||
tags?.Add(new KeyValuePair<string, object?>(ket, value)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I see only
db.client.operation.duration
is supported.