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

Azure Event Hub receiver Semantic Convention translator #32486

Closed
wants to merge 18 commits into from

Conversation

ThatRendle
Copy link
Contributor

Description:

This PR changes the translator used by the Azure Event Hubs receiver to change property names on log records and metrics to the equivalent OTel Semantic Conventions names, making it easier to query logs and metrics ingested in this way alongside other logs and metrics.

Testing: WIP.

Documentation: WIP

pkg/translator/azure/property_names.go Outdated Show resolved Hide resolved
pkg/translator/azure/property_names.go Outdated Show resolved Hide resolved
pkg/translator/azure/property_names.go Outdated Show resolved Hide resolved
pkg/translator/azure/resourcelogs_to_logs.go Outdated Show resolved Hide resolved
pkg/translator/azure/resourcelogs_to_logs.go Outdated Show resolved Hide resolved
Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

Please add some new test scenarios

pkg/translator/azure/resourcelogs_to_logs.go Outdated Show resolved Hide resolved
@cparkins
Copy link
Contributor

I like this idea and I think it makes a lot of sense to try to map to Semantic Conventions were possible. Can you make sure an Issue is created to track this against?

pkg/translator/azure/property_names.go Outdated Show resolved Hide resolved
pkg/translator/azure/property_names.go Outdated Show resolved Hide resolved
pkg/translator/azure/property_names.go Outdated Show resolved Hide resolved
pkg/translator/azure/property_names.go Outdated Show resolved Hide resolved
pkg/translator/azure/property_names.go Outdated Show resolved Hide resolved
pkg/translator/azure/property_names.go Outdated Show resolved Hide resolved
pkg/translator/azure/property_names.go Outdated Show resolved Hide resolved
@atoulme
Copy link
Contributor

atoulme commented Apr 24, 2024

Can you please also contribute those transformations to https://github.com/open-telemetry/semantic-conventions so we have it in the spec?

@ThatRendle ThatRendle marked this pull request as ready for review April 30, 2024 12:48
@ThatRendle ThatRendle requested a review from a team April 30, 2024 12:48
Copy link
Member

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

This looks great and will greatly improve consistency for Azure events - thanks @markrendle 🚀

@crobert-1
Copy link
Member

Going along with @atoulme's request, I think it may be good to block this PR until the suggested conventions here have been approved and merged into the semantic conventions repo, as any changes requested there will need to be reflected in this package (and PR). The problem with going forward with this PR before semantic conventions are finalized, would be the decisions there could end up being breaking changes here (if this gets merged and released).

Let me know if I've misunderstood the nature of this change though 👍

@TylerHelmuth
Copy link
Member

@atoulme @crobert-1 @cparkins I will get a semconv issue/PR opened to add the net-new attribute names. As for the mapping of azure fields to existing fields, I do not believe the semantic convention currently defines those. The closest I know about is are these appendices . I will open a issue/PR for that as well.

@MikeGoldsmith
Copy link
Member

This is still a WIP. The work to add semmantic conventions for the new azure namespace is on going.

@MovieStoreGuy
Copy link
Contributor

Is it worth converting this PR to draft until it is ready for review?

@github-actions github-actions bot removed the Stale label May 22, 2024
@MikeGoldsmith
Copy link
Member

Yep, I think that's wise given it's blocked on semantic conventions PR:

@MikeGoldsmith
Copy link
Member

Note to add opt-in for identity field as it contains PII data.

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jun 13, 2024
@MikeGoldsmith
Copy link
Member

We still want this PR to land, it's just waylaid by semconv work. The current direction looks like we'll move this translator towards using a defined event structure instead of attributes but not confirmed.

Is there a label we can apply to indicate this isn't stale but work in progress?

@crobert-1 crobert-1 removed the Stale label Jun 13, 2024
@crobert-1
Copy link
Member

There isn't a good label for that, but I can add never stale for now.

@crobert-1 crobert-1 added the never stale Issues marked with this label will be never staled and automatically removed label Jun 13, 2024
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@MikeGoldsmith
Copy link
Member

As an update, the semconv PR has picked up again and we'll be moving to an event model instead of a generic log. Named events have the benefit of a clear schema of field names where logs do not. Hopefully that PR will wrap up soon and we can update this work to reflect the changes.

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jul 18, 2024
@crobert-1 crobert-1 removed the Stale label Jul 18, 2024
codeboten pushed a commit that referenced this pull request Jul 18, 2024
This PR makes the `never stale` label respected on PRs. I noticed on
#32486
that I had added the `never stale` label, but `Stale` was still added.
This is because we need to set the [`exempt-pr-labels`
option](https://github.com/actions/stale?tab=readme-ov-file#exempt-pr-labels)
in the [stale action](https://github.com/actions/stale) workflow.
codeboten added a commit that referenced this pull request Sep 18, 2024
**Description**:

This PR adds a new Azure Resource Logs translator that can be used to
convert Azure events into resource logs events using Semantic
conventions defined here
[here](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/azure/events.md).

This supersedes following draft PR by updating it to have the latest
semconv:
- #32486

*Note*: A follow-up PR will update the Azure receiver to use the new
translator.

**Testing**:

Includes unit tests to verify expected behaviours and data structures.

**Documentation**:

---

cc @markrendle @lmolkova @TylerHelmuth

---------

Signed-off-by: Alex Boten <[email protected]>
Co-authored-by: Mark Rendle <[email protected]>
Co-authored-by: Alex Boten <[email protected]>
Co-authored-by: Curtis Robert <[email protected]>
Co-authored-by: Tyler Helmuth <[email protected]>
@MikeGoldsmith
Copy link
Member

This PR can be closed now #34830 has merged.

@codeboten
Copy link
Contributor

This PR can be closed now #34830 has merged.

👍🏻

@codeboten codeboten closed this Sep 19, 2024
jriguera pushed a commit to springernature/opentelemetry-collector-contrib that referenced this pull request Oct 4, 2024
**Description**:

This PR adds a new Azure Resource Logs translator that can be used to
convert Azure events into resource logs events using Semantic
conventions defined here
[here](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/azure/events.md).

This supersedes following draft PR by updating it to have the latest
semconv:
- open-telemetry#32486

*Note*: A follow-up PR will update the Azure receiver to use the new
translator.

**Testing**:

Includes unit tests to verify expected behaviours and data structures.

**Documentation**:

---

cc @markrendle @lmolkova @TylerHelmuth

---------

Signed-off-by: Alex Boten <[email protected]>
Co-authored-by: Mark Rendle <[email protected]>
Co-authored-by: Alex Boten <[email protected]>
Co-authored-by: Curtis Robert <[email protected]>
Co-authored-by: Tyler Helmuth <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
never stale Issues marked with this label will be never staled and automatically removed pkg/translator/azure receiver/azureeventhub receiver/kafka
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants