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

jmx metrics extension: Initial subprocess manager #1028

Merged
merged 5 commits into from
Sep 29, 2020
Merged

jmx metrics extension: Initial subprocess manager #1028

merged 5 commits into from
Sep 29, 2020

Conversation

rmfitzpatrick
Copy link
Contributor

Description:
Adding a feature - adding a subprocess manager to the jmx metrics extension that will be capable of managing child java processes. This is largely a port of https://github.com/open-telemetry/opentelemetry-collector/blob/master/extension/fluentbitextension/process.go and could potentially be relocated to collector core to satisfy open-telemetry/opentelemetry-collector#1492 in the future.

Link to tracking Issue:

Testing: Integration and unit tests

Documentation: None required, other than changelog that will occur later.

@rmfitzpatrick rmfitzpatrick requested a review from a team as a code owner September 14, 2020 16:25
@codecov
Copy link

codecov bot commented Sep 14, 2020

Codecov Report

Merging #1028 into master will increase coverage by 16.07%.
The diff coverage is 96.55%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1028       +/-   ##
===========================================
+ Coverage   73.67%   89.74%   +16.07%     
===========================================
  Files          22      275      +253     
  Lines        1018    13510    +12492     
===========================================
+ Hits          750    12125    +11375     
- Misses        217     1016      +799     
- Partials       51      369      +318     
Flag Coverage Δ
#integration 76.01% <93.10%> (+2.33%) ⬆️
#unit 88.88% <96.55%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...nsion/jmxmetricsextension/subprocess/subprocess.go 96.52% <96.52%> (ø)
...jmxmetricsextension/subprocess/subprocess_linux.go 100.00% <100.00%> (ø)
receiver/kubeletstatsreceiver/kubelet/fs.go 100.00% <0.00%> (ø)
receiver/carbonreceiver/protocol/regex_parser.go 97.22% <0.00%> (ø)
...asticexporter/internal/translator/elastic/utils.go 83.33% <0.00%> (ø)
...sformprocessor/operation_aggregate_label_values.go 100.00% <0.00%> (ø)
...etectionprocessor/internal/aws/ec2/metadata_ec2.go 100.00% <0.00%> (ø)
exporter/signalfxexporter/config.go 90.69% <0.00%> (ø)
receiver/collectdreceiver/receiver.go 71.23% <0.00%> (ø)
receiver/carbonreceiver/receiver.go 95.55% <0.00%> (ø)
... and 258 more

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 e3493ce...cd71b56. Read the comment docs.

@flands flands added this to Review requested in Collector Sep 17, 2020
@rmfitzpatrick
Copy link
Contributor Author

@bogdandrutu @tigrannajaryan any chance one of you could take a look at this?

@tigrannajaryan
Copy link
Member

Adding a feature - adding a subprocess manager to the jmx metrics extension that will be capable of managing child java processes. This is largely a port of https://github.com/open-telemetry/opentelemetry-collector/blob/master/extension/fluentbitextension/process.go and could potentially be relocated to collector core to satisfy open-telemetry/opentelemetry-collector#1492 in the future.

This is now the 3rd instance of a subprocess manager code, right? Perhaps time to actually make it a lib?

@rmfitzpatrick
Copy link
Contributor Author

@tigrannajaryan this subprocess manager doesn't have robust restart configuration or a persistent stdin mechanism (just writes once). I'd be happy to begin the process of porting this over to collector core but would really appreciate not being blocked on this extension's actual implementation by that work.

Collector automation moved this from Review requested to Reviewer approved Sep 29, 2020
@tigrannajaryan tigrannajaryan merged commit 25d5e48 into open-telemetry:master Sep 29, 2020
Collector automation moved this from Reviewer approved to Done Sep 29, 2020
codeboten pushed a commit that referenced this pull request Nov 23, 2022
Here is an example snippet that will not report tracing without this patch:

with psycopg2.connect(...) as conn, conn.cursor() as cursor:
    cursor.execute("select 1;")

Co-authored-by: Carl Bordum Hansen <carl@bordum.dk>
codeboten pushed a commit that referenced this pull request Nov 23, 2022
Here is an example snippet that will not report tracing without this patch:

with psycopg2.connect(...) as conn, conn.cursor() as cursor:
    cursor.execute("select 1;")

Co-authored-by: Carl Bordum Hansen <carl@bordum.dk>
codeboten pushed a commit that referenced this pull request Nov 23, 2022
Here is an example snippet that will not report tracing without this patch:

with psycopg2.connect(...) as conn, conn.cursor() as cursor:
    cursor.execute("select 1;")

Co-authored-by: Carl Bordum Hansen <carl@bordum.dk>
codeboten pushed a commit that referenced this pull request Nov 23, 2022
Here is an example snippet that will not report tracing without this patch:

with psycopg2.connect(...) as conn, conn.cursor() as cursor:
    cursor.execute("select 1;")

Co-authored-by: Carl Bordum Hansen <carl@bordum.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Collector
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants