Skip to content
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

Refactor OpenTelemetryData to Action<Activity> #143

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

stevejgordon
Copy link
Collaborator

Closes #142

This allows consumers such as ingest to record the actual number of operations.
Copy link
Member

@Mpdreamz Mpdreamz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More flexible 👍 lgtm.

@stevejgordon stevejgordon merged commit efe6b75 into main Nov 20, 2024
5 checks passed
@stevejgordon stevejgordon deleted the refactor/opentelemetry branch November 20, 2024 16:35
@flobernd flobernd added enhancement New feature or request v0.5.4 labels Nov 21, 2024
if (attribute.Key.Equals(OpenTelemetryAttributes.DbElasticsearchSchemaUrl, StringComparison.Ordinal))
{
if (attribute.Value is string schemaVersion)
productSchemaVersion = schemaVersion;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevejgordon Removing this causes the condition in line 69 to be always true.

/// <summary>
/// The name to use for spans relating to a request.
/// </summary>
public string? SpanName { get; init; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevejgordon How am I supposed to set a custom OperationName with the new design? The property has no setter. Should I use DisplayName to modify it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: DisplayName does not update the original OperationName which causes a test to fail in the client library. We have to re-introduce a way to provide the OperationName.

A potential solution would be to bring back OpenTelemetryData, but modify it to, besides OperationName, have the current Action<> as a property instead of the previous "attributes" dictionary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v0.5.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor OpenTelemetryData to Action<Activity>
3 participants