[transator/azurelogs] Add support for the remaining activity logs categories #44871
Conversation
Includes support for: - Administrative - Alert - Autoscale - Policy - Resource Health - Security - Service Health
zmoog
force-pushed
the
zmoog/pkg/translator/azurelogs/activitylogs-complete
branch
from
e84b5d8 to
f452454
Compare
github-actions
bot
requested review from
MikeGoldsmith,
constanca-m and
cparkins
| | `properties.category` | `azure.servicehealth.impact.category` | | ||
| | `properties.defaultLanguageTitle` | `azure.servicehealth.default_language.title` | | ||
| | `properties.defaultLanguageContent` | `azure.servicehealth.default_language.content` | | ||
| | `properties.stage` | `azure.servicehealth.stage` | |
There was a problem hiding this comment.
Could this be described as a state?
There was a problem hiding this comment.
The semantics of this field is not 100% clear yet, but "state" seems to fit. Do you think it's worth diverging from the field name?
|
Hey @thompson-tomo, thank you for kicking off the review! All the suggestions I see make sense. What are the best resources to get started with naming conventions in semconv, so I can avoid making n00b choices? |
Co-authored-by: James Thompson <thompson.tomo@outlook.com>
|
No worries at all. A good place to start would be https://opentelemetry.io/docs/specs/semconv/general/naming/ & https://opentelemetry.io/docs/specs/semconv/how-to-write-conventions/ there are tasks to improve those docs. Also a good understanding can be gained by reviewing what is already defined. The way I like to think during attribute naming is what is the item that is being described. That becomes the 2nd last namespace with the last being a property of the item you are capturing. |
| | `properties.service` | `azure.servicehealth.service` | | ||
| | `properties.region` | `azure.servicehealth.region` | | ||
| | `properties.communication.communicationId` | `azure.servicehealth.communication.id` | | ||
| | `properties.communication.communicationBody` | `azure.servicehealth.communication.text` | |
There was a problem hiding this comment.
How about just properties.communication?
There was a problem hiding this comment.
Uhm, we have azure.servicehealth.communication.id so I was considering moving the actual communication text under azure.servicehealth.communication.*: do you think this makes sense?
There was a problem hiding this comment.
I think that make sense! But I was looking at the sample log, I thought I see field from the sample log is just properties.communication instead of properties.communication.communicationBody?
There was a problem hiding this comment.
For me azure.servicehealth.communication.body would be the best attribute naming. Note in semantic conventions complex attributes ie objects are discouraged hence preferred to create individual attributes.
There was a problem hiding this comment.
azure.servicehealth.communication.body LGTM!
5 participants
Description
Includes support for all the remaining seven Activity log categories:
The PR is built on top of the PR #43220 that added support for the Recommendation log category.
It applies the same philosophy:
Note for reviewers: I'm not 100% on the field naming convention, so your feedback and insight are really needed here.
Link to tracking issue
N/A
Testing
I collected some sample logs from actual Azure and anonymized a little bit for the following log categories:
However, I wasn't able to get actual log events for the following categories, so I inferred them from samples available in the Azure docs (which seems to use the Log Analytics Workspace format):
Please pay extra attention when reviewing these categories, and please let me know if you're able to share some actual samples from Azure.
Documentation
N/A