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

feat: Send plugin logs via OTEL #1807

Merged
merged 3 commits into from
Jul 22, 2024
Merged

Conversation

erezrokah
Copy link
Member

@erezrokah erezrokah commented Jul 19, 2024

Summary

Still WIP, opening for reference


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines πŸ§‘β€πŸŽ“
  • Run go fmt to format your code πŸ–Š
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests πŸ§ͺ
  • Ensure the status checks below are successful βœ…

record.SetSeverity(otellogSeverity(level))
record.SetBody(otellog.StringValue(message))
// See https://github.com/rs/zerolog/issues/493, this is ugly but it works
logData := make(map[string]any)
Copy link
Member Author

Choose a reason for hiding this comment

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

Opened rs/zerolog#682 let's see how that goes πŸ˜„

Copy link
Contributor

Choose a reason for hiding this comment

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

😱 rs/zerolog#493 (comment) 🀣

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you just add a general comment (not about the hack, but about what's going on with zerolog + otel? It's very clear with the context of this PR but the code looks complex so it's not clear on its own I think.

record.SetSeverity(otellogSeverity(level))
record.SetBody(otellog.StringValue(message))
// See https://github.com/rs/zerolog/issues/493, this is ugly but it works
logData := make(map[string]any)
Copy link
Contributor

Choose a reason for hiding this comment

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

😱 rs/zerolog#493 (comment) 🀣

record.SetSeverity(otellogSeverity(level))
record.SetBody(otellog.StringValue(message))
// See https://github.com/rs/zerolog/issues/493, this is ugly but it works
logData := make(map[string]any)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you just add a general comment (not about the hack, but about what's going on with zerolog + otel? It's very clear with the context of this PR but the code looks complex so it's not clear on its own I think.

@erezrokah
Copy link
Member Author

Thanks for the review, added a more detailed comment. Someone is reviewing my PR at rs/zerolog#682 but I wasn't able to understand if they are a maintainer or not.

@kodiakhq kodiakhq bot merged commit 9897b83 into cloudquery:main Jul 22, 2024
7 checks passed
kodiakhq bot pushed a commit that referenced this pull request Jul 22, 2024
πŸ€– I have created a release *beep* *boop*
---


## [4.51.0](v4.50.1...v4.51.0) (2024-07-22)


### Features

* Send plugin logs via OTEL ([#1807](#1807)) ([9897b83](9897b83))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
kodiakhq bot pushed a commit to cloudquery/cloudquery that referenced this pull request Jul 23, 2024
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.

2 participants