-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[chore] [receiver/datadog] Refactor translation files into internal package #34160
[chore] [receiver/datadog] Refactor translation files into internal package #34160
Conversation
receiver/datadogreceiver/receiver.go
Outdated
@@ -8,6 +8,7 @@ import ( | |||
"encoding/json" | |||
"errors" | |||
"fmt" | |||
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/datadogreceiver/internal/translator" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the linter would fail on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you tell us why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We split the imports into three sections: stdlib, third party dependencies, and local packages. I believe the CI failure should point to the command to run to fix it. I think we have a make target to fix it as well (make fmt, IIRC). Can you check if this is part of the contributing guidelines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes I think ci lints this indeed
I dont see this being part of the contributing guidelines:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -1,7 +1,7 @@ | |||
// Copyright The OpenTelemetry Authors | |||
// SPDX-License-Identifier: Apache-2.0 | |||
|
|||
package datadogreceiver // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/datadogreceiver" | |||
package translator // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/datadogreceiver/internal/translator" | |||
|
|||
import ( | |||
"go.opentelemetry.io/collector/pdata/pcommon" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both Batcher and DImensions could be unexported.
@@ -52,7 +23,7 @@ type SeriesList struct { | |||
Series []datadogV1.Series `json:"series"` | |||
} | |||
|
|||
func (mt *MetricsTranslator) translateMetricsV1(series SeriesList) pmetric.Metrics { | |||
func (mt *MetricsTranslator) TranslateSeriesV1(series SeriesList) pmetric.Metrics { | |||
bt := newBatcher() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Over time the same batchers will be created again and again. This is not for this PR but I wonder if an improvement for the receiver would be to have a pool of the maps we've seen in the past and reuse them instead of reallocating them on every request handling.
57e4195
to
75db101
Compare
Description:
This PR is a follow-up to #33957. It refactors the Datadog receiver files to remove internal methods and structures from the public API and into an internal directory.
Link to tracking Issue:
#18278
Testing:
This is a refactor, so no new unit tests have been added.