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

Feature/prod 2955/add traces to hz cache #12

Merged
merged 16 commits into from
Nov 27, 2024

Conversation

JoelNygren-Norce
Copy link
Contributor

No description provided.

{
public static class HzCacheDiagnostics
{
public const string HzCacheVersion = "0.0.9";

Choose a reason for hiding this comment

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

Surely this will be 0.0.10 once this is merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maby we should remove this as no one will keep it updated?

Copy link

@johan-bjerling johan-bjerling left a comment

Choose a reason for hiding this comment

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

Possibly change the version string though.

Copy link

@patrik-j-nilsson patrik-j-nilsson left a comment

Choose a reason for hiding this comment

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

I think it's a bit unnecessary to have a dependency on OpenTelemetry in this package, then we need to keep it up to date here as well.

The extension method is a nice idea, but I fear it could create more problems than it's worth.
It should be as simple as adding builder.AddSource(HzCacheDiagnostics.ActivitySourceName); in the service that uses this package.

removed Extesion
@JoelNygren-Norce JoelNygren-Norce merged commit cedfb53 into main Nov 27, 2024
2 checks passed
@JoelNygren-Norce JoelNygren-Norce deleted the feature/PROD-2955/Add-Traces-to-HzCache branch November 27, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants