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 attributes & Logs events #1028

Merged
merged 55 commits into from
Aug 1, 2024

Conversation

MikeGoldsmith
Copy link
Member

@MikeGoldsmith MikeGoldsmith commented May 10, 2024

Fixes #1027, #1299

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

model/registry/azure.yaml Outdated Show resolved Hide resolved
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
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 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 Outdated 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

@lmolkova I think this is ready for (hopefully) final review 😢

model/logs/azure.yaml Outdated Show resolved Hide resolved
docs/attributes-registry/azure.md Outdated Show resolved Hide resolved
model/logs/azure.yaml Outdated Show resolved Hide resolved
model/logs/azure.yaml Show resolved Hide resolved
@lmolkova
Copy link
Contributor

/cc @MSNev @trask if you want to take a look.

model/logs/azure.yaml Outdated Show resolved Hide resolved
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.

Left a couple of cosmetic comments, looks great otherwise!

docs/azure/events.md Show resolved Hide resolved
docs/azure/events.md Outdated Show resolved Hide resolved
@MikeGoldsmith
Copy link
Member Author

Updated properties to be a keyvaluelist field type.

@MikeGoldsmith
Copy link
Member Author

I've addressed feedback - @lmolkova @trask. Let me know if there's anything else you'd like to see.

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.

Thank you!

model/logs/azure.yaml Show resolved Hide resolved
MikeGoldsmith and others added 2 commits July 30, 2024 17:17
Co-authored-by: Liudmila Molkova <limolkova@microsoft.com>
@lmolkova lmolkova merged commit 798e939 into open-telemetry:main Aug 1, 2024
13 checks passed
@MikeGoldsmith MikeGoldsmith deleted the mike/azure-logs branch August 2, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

New Azure Logs area
7 participants