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 Resource Logs translator #34830

Merged
merged 60 commits into from
Sep 18, 2024

Conversation

MikeGoldsmith
Copy link
Member

@MikeGoldsmith MikeGoldsmith commented Aug 23, 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.

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

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

@MikeGoldsmith
Copy link
Member Author

I'm a little unsure why both the changelog is failing:
image

and why the receiver is failing with a missing go.sum entry
image

Any help would be appreciated 😄

@cparkins
Copy link
Contributor

@MikeGoldsmith I would remove the '[user]' from your change log entry and see if it fixes the issue since it says it's the default value anyway. I'll also look into the go mod issue, although I always struggle with those too!

@atoulme
Copy link
Contributor

atoulme commented Aug 23, 2024

single quotes around your array makes it a string '[user]'

@cparkins
Copy link
Contributor

@MikeGoldsmith I submitted a PR to your fork to try and address some of the build issues.

@MikeGoldsmith
Copy link
Member Author

Thank you @cparkins, I'll take a look shortly.

Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@TylerHelmuth TylerHelmuth added the ready to merge Code review completed; ready to merge by maintainers label Sep 17, 2024
@TylerHelmuth TylerHelmuth removed the ready to merge Code review completed; ready to merge by maintainers label Sep 18, 2024
@codeboten codeboten requested a review from a team as a code owner September 18, 2024 22:18
Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten codeboten merged commit 3c8a507 into open-telemetry:main Sep 18, 2024
155 of 156 checks passed
@github-actions github-actions bot added this to the next release milestone Sep 18, 2024
@MikeGoldsmith MikeGoldsmith deleted the azureeventhub_otelsc branch September 18, 2024 23:43
@an-mmx
Copy link
Contributor

an-mmx commented Sep 26, 2024

Hey @MikeGoldsmith ,

Sorry for the late reply. It seems i've missed some updates.
I've seen your changes for properties type check and this probably a good solution. The properties of FunctionAppLogs is now stored as string attribute.

I believe it would be a good in next step to introduce the possibility to enable corrections to JSON string so that it would be possible to apply semantic conventions translation. We do it unconditionally in our implementation (you can check it in this commit). I agree, It's not the best approach. It worth to add some extra logic to make it optional and keep the properties in attributes when failed to parse JSON. Nevertheless it allows us to apply translations. I'd like to hear your thoughts on this.

There is also another commit with the time convertion approach. I would like to create a PR with these changes for the both azure and azurelogs translators, after #35357 is being merged. Of curse, if no objections?

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 <223565+codeboten@users.noreply.github.com>
Co-authored-by: Mark Rendle <mark@rendlelabs.com>
Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Co-authored-by: Curtis Robert <crobert@splunk.com>
Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com>
andrzej-stencel pushed a commit that referenced this pull request Oct 16, 2024
**Description:**

Updates the AzureEventsHubReceiver to use the new azure resource logs
translator.

Includes the following changes:
- Add config option to enable the new behaviour, default is off so
requires users to opt-in
- Add interface to allow switching over the previous and new translator
logic
- Update receiver to use new config option to return the appropriate
translator
- Update README with new configuration option

Follow-up PR to adding the translator:
- #34830 

**Link to tracking Issue:**
N/A

**Testing:**
Includes unit test to verify configuration defaults and changes.

**Documentation:**
N/A

---------

Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
…-telemetry#35357)

**Description:**

Updates the AzureEventsHubReceiver to use the new azure resource logs
translator.

Includes the following changes:
- Add config option to enable the new behaviour, default is off so
requires users to opt-in
- Add interface to allow switching over the previous and new translator
logic
- Update receiver to use new config option to return the appropriate
translator
- Update README with new configuration option

Follow-up PR to adding the translator:
- open-telemetry#34830 

**Link to tracking Issue:**
N/A

**Testing:**
Includes unit test to verify configuration defaults and changes.

**Documentation:**
N/A

---------

Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants