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

Implement VMMetrics factory and config V2 #90

Merged

Conversation

tigrannajaryan
Copy link
Member

Github issue: #35

Testing done: make && make otelsvc

Github issue: open-telemetry#35

Testing done: make && make otelsvc
Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

pjanotti referenced this pull request in pjanotti/opentelemetry-service Jul 2, 2019
Categorize namespaces of each item into:

- exporters
- interceptors
- zpages

which aesthetically looks nicer than the original but also
gives users better context and control over what the agent
does.

For example here is the new configuration file:
```yaml
interceptors:
    opencensus:
        address: "127.0.0.1:55678"

exporters:
    stackdriver:
        project: "project-id"
        enable_tracing: true

    zipkin:
        endpoint: "http://localhost:9411/api/v2/spans"

    datadog:
        enable_tracing: false

zpages:
    port: 55679
```

Fixes #90
@tigrannajaryan
Copy link
Member Author

LGTM. Do you want to clean up the old config?

@songy23 yes, it is in my todo list. I just want to first have feature parity in new config implementation and then start deleting old config support.

@tigrannajaryan
Copy link
Member Author

Keeping the support for old config allows to build old occollector executable and run it and compare with otelsvc behavior. Very useful for testing.

// StartMetricsReception scrapes VM metrics based on the OS platform.
func (vmr *Receiver) StartMetricsReception(ctx context.Context, asyncErrorChan chan<- error) error {
func (vmr *Receiver) StartMetricsReception(host receiver.Host) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@4979bbd). Click here to learn what that means.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #90   +/-   ##
=========================================
  Coverage          ?   69.87%           
=========================================
  Files             ?       98           
  Lines             ?     6304           
  Branches          ?        0           
=========================================
  Hits              ?     4405           
  Misses            ?     1669           
  Partials          ?      230
Impacted Files Coverage Δ
receiver/vmmetricsreceiver/metrics_receiver.go 0% <0%> (ø)
receiver/vmmetricsreceiver/factory.go 90% <90%> (ø)

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 4979bbd...229d228. Read the comment docs.

@tigrannajaryan tigrannajaryan merged commit c50224d into open-telemetry:master Jul 2, 2019
@tigrannajaryan tigrannajaryan deleted the feature/tigran/vmmetrics branch July 2, 2019 23:33
@tigrannajaryan
Copy link
Member Author

Results as seen in Prometheus:

image

hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
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