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

Add Azure SDK & Logs #1028

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

MikeGoldsmith
Copy link
Member

@MikeGoldsmith MikeGoldsmith commented May 10, 2024

Fixes #1027

Changes

Adds the new Azure Logs semantic conventions.

Merge requirement checklist

filled attribute descriptions and examples

add changelog

clean-up yaml to pass linter

add initial description for azure logs markdown

more linter appeasing

rename azure logs md
@MikeGoldsmith MikeGoldsmith changed the title Add Azure Logs Add Azure SDK & Logs May 14, 2024
@MikeGoldsmith
Copy link
Member Author

Hi @lmolkova - please can we get your input on adding these Azure SDK / logs semantic conventions? Thanks 😄

@lmolkova
Copy link
Contributor

lmolkova commented May 24, 2024

Thank you for for working on this!

I'll share more comments, but the main question is whether we need to define those as attributes.
My assumption that azure logs would be mapped to OTel logs/log-based-events.

Logs/events have attributes and payload. We can take azure log as is, document the payload format and send them as is over OTLP with no mapping.

Event payload fields can be documented, here's an example, they don't need a namespace.

If there are no use-case for these attributes to appear on spans, metrics, resources, then event payload fields would work better.

So, was this option was considered? What would be the reasons to send azure logs with those as attributes and not event payload fields?

/cc @MSNev

Copy link
Contributor

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

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

I believe there are two major discussion points:

  • whether we need attributes for azure log events or they should be populated as event payload fields (elaborated in the comment above)
  • how the actual mapping works - is it possible to document Azure Log Event semconv? What goes into event name, timestamp, severity, payload, attributes, etc

model/registry/azure.yaml Outdated Show resolved Hide resolved
model/registry/azure.yaml Outdated Show resolved Hide resolved
- AuditEvent
- GatewayLogs
- ApplicationGatewayAccessLog
- id: correlation.id
Copy link
Contributor

Choose a reason for hiding this comment

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

no strong opinion, but wonder if we should reuse az.service_request_id here.

model/registry/azure.yaml Outdated Show resolved Hide resolved
- "{'int': 1234 }"
- "{'float': '1.234'}"
- "{'bool': true}"
- id: resource.id
Copy link
Contributor

Choose a reason for hiding this comment

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

we should reuse existing cloud.resource_id

model/registry/azure.yaml Outdated Show resolved Hide resolved
model/registry/azure.yaml Outdated Show resolved Hide resolved
model/registry/azure.yaml Outdated Show resolved Hide resolved
model/registry/azure.yaml Show resolved Hide resolved
model/registry/azure.yaml Outdated Show resolved Hide resolved
@TylerHelmuth
Copy link
Member

@lmolkova the PR went with attributes because that has been the standard for collector translators/components so far. I believe the k8sobjectsreceiver is an early implementor of the event concept, but is currently suffering for being an early adopter (it sets event.domain still which was removed).

I'm not very up-to-date with the event spec. I see it is marked as experimental still, is it changing a lot still? I see this PR as an attempt to provide some clarity/stability for azure -> OTel semantics and I'm wondering if attributes gives more stability than events while azure -> OTel semantics become stable.

@TylerHelmuth
Copy link
Member

Another question:

My understanding is that for any event.name value, the payload (log body in this case) should always be same. Do we have that guarantee with the data being targeted in open-telemetry/opentelemetry-collector-contrib#32486?

MikeGoldsmith and others added 5 commits May 29, 2024 14:27
Co-authored-by: Liudmila Molkova <limolkova@microsoft.com>
Co-authored-by: Liudmila Molkova <limolkova@microsoft.com>
Co-authored-by: Liudmila Molkova <limolkova@microsoft.com>
@MikeGoldsmith
Copy link
Member Author

Thanks for your feedback @lmolkova. I've worked through most of your suggestions.

Regarding reuse of az.service_request_id and cloud.resource_id fields, do you mean using a link feature between two semconv definitions (I'm unsure if that exists or how to do it), or do you mean use the same field and description as the linked fields?

@MikeGoldsmith MikeGoldsmith marked this pull request as ready for review May 29, 2024 16:06
@MikeGoldsmith MikeGoldsmith requested review from a team as code owners May 29, 2024 16:06
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.

New Azure Logs area
6 participants