-
Notifications
You must be signed in to change notification settings - Fork 852
#2205 data about db & messaging checks #2228
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
base: master
Are you sure you want to change the base?
#2205 data about db & messaging checks #2228
Conversation
43fbd6d to
df83cf1
Compare
|
@sungam3r &/or @adamsitnik any chance you could take a look please. Change is focused on adding metadata to the health reports so we can know what the report is for etc. |
|
|
||
| foreach (var (topicName, subscriptions) in _snsOptions.TopicsAndSubscriptions.Select(x => (x.Key, x.Value))) | ||
| { | ||
| currentTopic = topicName; |
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.
Can only be added to unhealthy due to checking multiple topic/Subscription.
|
|
||
| foreach (string? subscription in subscriptions) | ||
| { | ||
| currentSubscription = subscription; |
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.
Can only be added to unhealthy due to checking multiple Subscription.
| using var client = CreateSqsClient(); | ||
| foreach (var queueName in _sqsOptions.Queues) | ||
| { | ||
| currentQueue = queueName; |
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.
Can only be added to unhealthy due to checking multiple queue.
28af2f2 to
77a79ce
Compare
| public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default) | ||
| { | ||
| var checkDetails = new Dictionary<string, object>{ | ||
| { "health_check.task", "ready" }, |
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.
the health check conventions (spans? events? metrics?) is a great candidate to be defined in semantic conventions. We'd probably have a discussion on whether health_check operation is also a messaging/db/etc and it would need a project/group to own and prototype it across different frameworks.
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.
All of the signal types with semconv issue already existing open-telemetry/semantic-conventions#1106
What this PR does / why we need it:
With these changes the health entries will now have the data attribute populated with some attributes which are keyed following OTLP semantic conventions. These can be used in otel events via a publisher.
Which issue(s) this PR fixes:
Closes #2205
Closes #2227
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Please make sure you've completed the relevant tasks for this PR, out of the following list: