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

metrics transform processor README #339

Closed

Conversation

JingboWangGoogle
Copy link
Contributor

Description: README for the metrics transform processor

Link to tracking Issue: #332 (comment)

Documentation: preview is available here: https://github.com/JingboWangGoogle/opentelemetry-collector-contrib/tree/readme/processor/metricstransformprocessor

@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #339 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #339   +/-   ##
=======================================
  Coverage   83.38%   83.38%           
=======================================
  Files         167      167           
  Lines        8943     8943           
=======================================
  Hits         7457     7457           
  Misses       1161     1161           
  Partials      325      325           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ca134c...d65a0ee. Read the comment docs.

Copy link
Contributor

@draffensperger draffensperger left a comment

Choose a reason for hiding this comment

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

LGTM from my perspective.

@JingboWangGoogle JingboWangGoogle marked this pull request as ready for review June 17, 2020 19:56
@JingboWangGoogle JingboWangGoogle requested a review from a team June 17, 2020 19:56
processor/metricstransformprocessor/README.md Show resolved Hide resolved
processor/metricstransformprocessor/README.md Outdated Show resolved Hide resolved
processor/metricstransformprocessor/README.md Outdated Show resolved Hide resolved
processor/metricstransformprocessor/README.md Outdated Show resolved Hide resolved
processor/metricstransformprocessor/README.md Outdated Show resolved Hide resolved
processor/metricstransformprocessor/README.md Outdated Show resolved Hide resolved
processor/metricstransformprocessor/README.md Outdated Show resolved Hide resolved
processor/metricstransformprocessor/README.md Outdated Show resolved Hide resolved
@JingboWangGoogle JingboWangGoogle changed the title metrics transform processor README draft metrics transform processor README Jun 22, 2020
@tigrannajaryan
Copy link
Member

Does this add documentation for a processor that does not exist in this repo yet?

@JingboWangGoogle
Copy link
Contributor Author

JingboWangGoogle commented Jun 22, 2020

@tigrannajaryan This adds documentation and establishes a space for an incoming processor that is still in progress. As suggested by @bogdandrutu from a meeting, initializing a space with a README first would be a good first step.

@JingboWangGoogle
Copy link
Contributor Author

JingboWangGoogle commented Jun 24, 2020

Hi @asuresh4 , I am a Google intern currently working on this project (issue proposal), and I see that you are assigned to this Pull Request. Since I am not seeing many activities here, do you think you could merge this anytime soon, and after that, my subsequent Pull Request will also come into this folder for this metrics transform processor.


## Examples

### Insert New Metric
Copy link
Member

Choose a reason for hiding this comment

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

What would the config look like if I'd like to rename more than one metric? I maybe missing the context, but it appears to me like one can only operate on a single metric per processor config. Is that correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, it was actually a bit out of date. I just updated the new version, which has a transforms field that accepts a list, which can be used to transform more than one metric.

Copy link
Member

Choose a reason for hiding this comment

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

Now that your other PR is quite far along, I recommend you just incorporate this README into your other PR and close this one. We can merge this together with the implementation once that's ready

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea that works too!

@JingboWangGoogle
Copy link
Contributor Author

README moved to other PR: #347

@JingboWangGoogle JingboWangGoogle deleted the readme branch July 7, 2020 15:05
mxiamxia referenced this pull request in mxiamxia/opentelemetry-collector-contrib Jul 22, 2020
bogdandrutu pushed a commit that referenced this pull request May 12, 2022
Bumps [go.uber.org/zap](https://github.com/uber-go/zap) from 1.19.1 to 1.20.0.
- [Release notes](https://github.com/uber-go/zap/releases)
- [Changelog](https://github.com/uber-go/zap/blob/master/CHANGELOG.md)
- [Commits](uber-go/zap@v1.19.1...v1.20.0)

---
updated-dependencies:
- dependency-name: go.uber.org/zap
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.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.

5 participants