8000 feat(opentelemetry-configuration): creation of basic FileConfigProvider by maryliag · Pull Request #5863 · open-telemetry/opentelemetry-js · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
### :rocket: Features

*feat(opentelemetry-configuration): creation of basic ConfigProvider [#5809](https://github.com/open-telemetry/opentelemetry-js/pull/5809) @maryliag
*feat(opentelemetry-configuration): creation of basic FileConfigProvider [#5863](https://github.com/open-telemetry/opentelemetry-js/pull/5863) @maryliag

### :bug: Bug Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@

import { ConfigProvider } from './IConfigProvider';
import { EnvironmentConfigProvider } from './EnvironmentConfigProvider';
import { FileConfigProvider, hasValidConfigFile } from './FileConfigProvider';

export function createConfigProvider(): ConfigProvider {
if (hasValidConfigFile()) {
return new FileConfigProvider();
}
return new EnvironmentConfigProvider();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { getStringFromEnv } from '@opentelemetry/core';
import {
ConfigurationModel,
initializeDefaultConfiguration,
} from './configModel';
import { ConfigProvider } from './IConfigProvider';
import * as fs from 'fs';

export class FileConfigProvider implements ConfigProvider {
private _config: ConfigurationModel;

constructor() {
this._config = initializeDefaultConfiguration();
}

getInstrumentationConfig(): ConfigurationModel {
return this._config;
}
}

export function hasValidConfigFile(): boolean {
const configFile = getStringFromEnv('OTEL_EXPERIMENTAL_CONFIG_FILE');
if (configFile) {
if (!configFile.endsWith('.yaml') || !fs.existsSync(configFile)) {
throw new Error(
`Config file ${configFile} set on OTEL_EXPERIMENTAL_CONFIG_FILE is not valid`
);
}
return true;
}
return false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,45 @@ describe('ConfigProvider', function () {
);
});
});

describe('get values from config file', function () {
afterEach(function () {
delete process.env.OTEL_EXPERIMENTAL_CONFIG_FILE;
});

it('should initialize config with default values from valid config file', function () {
process.env.OTEL_EXPERIMENTAL_CONFIG_FILE =
'test/fixtures/kitchen-sink.yaml';
const configProvider = createConfigProvider();
assert.deepStrictEqual(
configProvider.getInstrumentationConfig(),
defaultConfig
);
});

it('should return error from invalid config file', function () {
process.env.OTEL_EXPERIMENTAL_CONFIG_FILE = './fixtures/kitchen-sink.txt';
assert.throws(() => {
createConfigProvider();
});
});

it('should initialize config with default values with empty string for config file', function () {
process.env.OTEL_EXPERIMENTAL_CONFIG_FILE = '';
const configProvider = createConfigProvider();
assert.deepStrictEqual(
configProvider.getInstrumentationConfig(),
defaultConfig
);
});

it('should initialize config with default values with all whitespace for config file', function () {
process.env.OTEL_EXPERIMENTAL_CONFIG_FILE = ' ';
const configProvider = createConfigProvider();
assert.deepStrictEqual(
configProvider.getInstrumentationConfig(),
defaultConfig
);
});
});
Copy link
Member

Choose a reason for hiding this comment

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

nit: I think it could be beneficial to also add some tests for OTEL_EXPERIMENTAL_CONFIG_FILE

  • unset
  • empty string
  • all whitespace

(should all not throw, looking at the code all cases are handled correctly right now already 🙂)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the unset case is already tested, since it what the cases the use env variables do, but good point on adding for the other two, let me do that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

});
Loading
Loading
0