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

[pkg/otlp/logs] Switch to Translator design #230

Merged
merged 3 commits into from
Dec 20, 2023

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Dec 19, 2023

What does this PR do?

Move to a Translator struct design for logs. This is part of the refactor for accomplishing #222, I need to do this so that:

  1. We can hold an attributes translator that has a counter inside the logs.Translator
  2. It's easier for me to refactor extracting the hostname from the resource. Right now we call SourceFromAttrs once per log record, we can just call it once per resource.

Motivation

@mx-psi mx-psi requested a review from a team as a code owner December 19, 2023 16:39
@mx-psi mx-psi requested review from dineshg13 and mackjmr December 19, 2023 16:39
Copy link
Member Author

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Note about the hideous diff: I renamed logs_translator.go to transform.go and then added the translator in logs_translator.go

@mx-psi
Copy link
Member Author

mx-psi commented Dec 19, 2023

@@ -15,251 +15,49 @@
package logs
Copy link
Member

Choose a reason for hiding this comment

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

Can u rename the file to translator.go ?

Copy link
Member

@dineshg13 dineshg13 left a comment

Choose a reason for hiding this comment

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

LGTM - can merge after fixing the file name

@mx-psi mx-psi merged commit eefd3ff into main Dec 20, 2023
7 checks passed
@mx-psi mx-psi deleted the mx-psi/attributes-translator branch December 20, 2023 12:23
codeboten pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Dec 21, 2023
**Description:** 

- [*/datadog] Bump to opentelemetry-mapping-go v0.11.0. This includes:
  - Refactor related to upcoming metric:
    - DataDog/opentelemetry-mapping-go/pull/230
    - DataDog/opentelemetry-mapping-go/pull/231
    - DataDog/opentelemetry-mapping-go/pull/229
  - Host metadata updates:
    - DataDog/opentelemetry-mapping-go/pull/184
    - DataDog/opentelemetry-mapping-go/pull/225
- Use `logs.Translator` for logs implementation
- Set `MeterProvider` to noop to prevent sending metrics for now until
we agree on the design. Note that I reverted one of the changelog items
added in #29785, since it's no longer true that we add this metric.
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Jan 10, 2024
…y#30158)

**Description:** 

- [*/datadog] Bump to opentelemetry-mapping-go v0.11.0. This includes:
  - Refactor related to upcoming metric:
    - DataDog/opentelemetry-mapping-go/pull/230
    - DataDog/opentelemetry-mapping-go/pull/231
    - DataDog/opentelemetry-mapping-go/pull/229
  - Host metadata updates:
    - DataDog/opentelemetry-mapping-go/pull/184
    - DataDog/opentelemetry-mapping-go/pull/225
- Use `logs.Translator` for logs implementation
- Set `MeterProvider` to noop to prevent sending metrics for now until
we agree on the design. Note that I reverted one of the changelog items
added in open-telemetry#29785, since it's no longer true that we add this metric.
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.

2 participants