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.

Sign up for GitHub

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

Jump to bottom

feat: process resource detector #1531

Merged
merged 7 commits into from
Sep 21, 2020
Merged

feat: process resource detector #1531

merged 7 commits into from
Sep 21, 2020

Conversation

mihirsoni
Copy link
Contributor

Which problem is this PR solving?

Adds process resource detectors.

Short description of the changes

This PR will add a new resource detectors for the process.

Resources are derived from following

Attribute Source Required
process.pid process.pid 1234
process.executable.name process.title See below
process.executable.path Skipped See below
process.command process.argv[0] See below
process.command_line process.argv See below
process.owner Skipped No

At least one of process.executable.name, process.executable.path, process.command, or process.command_line is required.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 14, 2020 •
edited

CLA Check
The committers are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Sep 14, 2020 •
edited

Codecov Report

Merging #1531 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1531      +/-   ##
==========================================
+ Coverage   93.28%   93.30%   +0.02%     
==========================================
  Files         153      154       +1     
  Lines        4750     4768      +18     
  Branches      950      958       +8     
==========================================
+ Hits         4431     4449      +18     
  Misses        319      319
Impacted Files Coverage Δ
packages/opentelemetry-sdk-node/src/sdk.ts 73.91% <ø> (ø)
packages/opentelemetry-resources/src/constants.ts 100.00% <100.00%> (ø)
...ces/src/platform/node/detectors/ProcessDetector.ts 100.00% <100.00%> (ø)

@mihirsoni mihirsoni marked this pull request as ready for review September 14, 2020 18:39
@dyladan dyladan added the enhancement New feature or request label Sep 21, 2020
@dyladan dyladan merged commit 0dbcba3 into open-telemetry:master Sep 21, 2020
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
@Zirak @blumamir
feat(auto-instrumentations-node): Expose getting resource detectors (o…
89f07d1 
…pen-telemetry#1772)

* feat(auto-instrumentations-node): Expose getting resource detectors

The auto instrumentation package provides users with a very easy way to
instrument their application, either from the commandline or
programatically. Users wishing the configure these instrumentations can also
call `getNodeAutoInstrumentations`.

This commit also exposes `getResourceDetectorsFromEnv`, so that users
configuring or somehow wishing to change the easy to use autoinstrumentation
entrypoint can also use the preconfigured resource detectors. Since the
autoinstrumentation package sets its exports, this function was previously
inaccessible.

Usage can look like this:

```js
const { getNodeAutoInstrumentations, getResourceDetectorsFromEnv } = require('@opentelemetry/auto-instrumentations-node');
const opentelemetry = require('@opentelemetry/sdk-node');

const sdk = new opentelemetry.NodeSDK({
  instrumentations: getNodeAutoInstrumentations({
    '@opentelemetry/instrumentation-fs': { enabled: false },
  }),
  resourceDetectors: getResourceDetectorsFromEnv(),
});

// use the sdk
```

Workarounds can be found [elsewhere in the opentelemetry repos](https://github.com/open-telemetry/opentelemetry-demo/blob/855bde3588b0fe85e500bec185dc2f311b15f98a/src/paymentservice/opentelemetry.js#L28-L38).
These can be fixed once a release is made.

Resolves open-telemetry#1531.

* style: Align indentation

---------

Co-authored-by: Amir Blum <amirgiraffe@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@obecny obecny obecny approved these changes

@dyladan dyladan dyladan approved these changes

@vmarchaud vmarchaud vmarchaud approved these changes

@markwolff markwolff markwolff approved these changes

@legendecas legendecas Awaiting requested review from legendecas

@mayurkale22 mayurkale22 Awaiting requested review from mayurkale22

@mwear mwear Awaiting requested review from mwear

@naseemkullah naseemkullah Awaiting requested review from naseemkullah

@OlivierAlbertini OlivierAlbertini Awaiting requested review from OlivierAlbertini

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Detect process resources
5 participants
@mihirsoni @obecny @dyladan @vmarchaud @markwolff