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 support for resource mapping in the Stackdriver exporter config #163

Merged
merged 3 commits into from
Apr 3, 2020
Merged

Add support for resource mapping in the Stackdriver exporter config #163

merged 3 commits into from
Apr 3, 2020

Conversation

nilebox
Copy link
Member

@nilebox nilebox commented Apr 1, 2020

Description:
Adding a feature - declarative config for mapping OpenCensus resources to Stackdriver resources.

Testing: Unit tests

Documentation: -

@nilebox
Copy link
Member Author

nilebox commented Apr 1, 2020

/cc @rghetia @x13n

Copy link
Contributor

@rghetia rghetia left a comment

Choose a reason for hiding this comment

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

minimum label check functionality is missing. For example, for a specific resource_type stackdriver expects that certain labels be present. Even if one of the label is not present then entire time-series request is rejected.
So, either all required mappings are mandatory OR create additional parameter for each label whether it is mandatory or not. If it is mandatory and the label is missing then it defaults to defaultMapper.

@nilebox
Copy link
Member Author

nilebox commented Apr 2, 2020

So, either all required mappings are mandatory OR create additional parameter for each label whether it is mandatory or not.

Made labels required by default, with extra optional parameter to make some of them optional.

Copy link
Contributor

@rghetia rghetia left a comment

Choose a reason for hiding this comment

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

LGTM except for one minor change in the comment.

Labels: map[string]string{
// Labels passed through with no changes
"source.label1": "value1",
"project_id": "123",
Copy link
Contributor

Choose a reason for hiding this comment

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

For a moment I was wondering why project_id got converted even though the other mandatory label is missing. But the project_id is converted by default implementation. Perhaps add this info in the comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added comments to clarify.

type LabelMapping struct {
SourceKey string `mapstructure:"source_key"`
TargetKey string `mapstructure:"target_key"`
Optional bool `mapstructure:"optional"`
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 document the semantics of this parameter? When would one set it to true?

Copy link
Member Author

Choose a reason for hiding this comment

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

It just signals that we can proceed with transformation when a particular label with a given source_key is missing.
I will add a comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@bogdandrutu bogdandrutu merged commit 839fe1c into open-telemetry:master Apr 3, 2020
@nilebox nilebox deleted the nilebox/stackdriver-map-resource-config branch April 3, 2020 02:59
mxiamxia referenced this pull request in mxiamxia/opentelemetry-collector-contrib Jul 22, 2020
The original PR was made before the change in model of factory registration. This change moves it to the current factory registration.
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* sdk/trace: Test the Sampling Probability

Closes #163

* Add tests for when the parent is sampled.
bogdandrutu pushed a commit that referenced this pull request May 12, 2022
Bumps [go.opentelemetry.io/collector](https://github.com/open-telemetry/opentelemetry-collector) from 0.26.0 to 0.27.0.
- [Release notes](https://github.com/open-telemetry/opentelemetry-collector/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-collector/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-collector@v0.26.0...v0.27.0)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request Feb 29, 2024
Co-authored-by: Antoine Toulme <antoine@lunar-ocean.com>
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.

4 participants