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

We should avoid exception when resource attributes pair value is empty #5769

Closed
erenming opened this issue Aug 25, 2023 · 3 comments
Closed
Labels
Feature Request Suggest an idea for this project

Comments

@erenming
Copy link

Is your feature request related to a problem? Please describe.
We are using otel to auto instrumentation with our app in k8s cluster.
And want to set deployment.environment to OTEL_RESOURCE_ATTRIBUTES, but the value will be empty sometimes.
When it's empty, the java app will throw exception.
Like the log of below, the keyvalue pair is deployment.environment=, the value is empty, so it throw an exception

Defaulted container "app" out of: app, opentelemetry-auto-instrumentation (init)
Picked up JAVA_TOOL_OPTIONS: -javaagent:/otel-auto-instrumentation/javaagent.jar
OpenJDK 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended
[otel.javaagent 2023-08-25 06:18:22:787 +0000] [main] INFO io.opentelemetry.javaagent.tooling.VersionLogger - opentelemetry-javaagent - version: 1.28.0
OpenTelemetry Javaagent failed to start
io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException: Invalid map property: otel.resource.attributes=sampler.ratio=1,service.instance.id=default/spring-petclinic-d5ffdc96c-j9brn,deployment.environment=,k8s.container.name=app,k8s.deployment.name=spring-petclinic,k8s.namespace.name=default,k8s.node.name=docker-desktop,k8s.pod.name=spring-petclinic-d5ffdc96c-j9brn,k8s.replicaset.name=spring-petclinic-d5ffdc96c,service.version=latest
at io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties.lambda$getMap$6(DefaultConfigProperties.java:217)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
at io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties.getMap(DefaultConfigProperties.java:224)
at io.opentelemetry.instrumentation.spring.resources.SpringBootServiceNameDetector.shouldApply(SpringBootServiceNameDetector.java:107)
at io.opentelemetry.sdk.autoconfigure.ResourceConfiguration.configureResource(ResourceConfiguration.java:101)
at io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder.build(AutoConfiguredOpenTelemetrySdkBuilder.java:335)
at io.opentelemetry.javaagent.tooling.OpenTelemetryInstaller.installOpenTelemetrySdk(OpenTelemetryInstaller.java:34)
at io.opentelemetry.javaagent.tooling.AgentInstaller.installBytebuddyAgent(AgentInstaller.java:121)
at io.opentelemetry.javaagent.tooling.AgentInstaller.installBytebuddyAgent(AgentInstaller.java:101)
at io.opentelemetry.javaagent.tooling.AgentStarterImpl.start(AgentStarterImpl.java:98)
at io.opentelemetry.javaagent.bootstrap.AgentInitializer$1.run(AgentInitializer.java:53)
at io.opentelemetry.javaagent.bootstrap.AgentInitializer$1.run(AgentInitializer.java:47)
at io.opentelemetry.javaagent.bootstrap.AgentInitializer.execute(AgentInitializer.java:64)
at io.opentelemetry.javaagent.bootstrap.AgentInitializer.initialize(AgentInitializer.java:46)
at io.opentelemetry.javaagent.OpenTelemetryAgent.startAgent(OpenTelemetryAgent.java:57)
at io.opentelemetry.javaagent.OpenTelemetryAgent.premain(OpenTelemetryAgent.java:45)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAgent(InstrumentationImpl.java:491)
at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPremain(InstrumentationImpl.java:503)

Describe the solution you'd like
we should keep the keyvalue pair even the value is empty

Describe alternatives you've considered

Additional context
the Instrumentation config

apiVersion: opentelemetry.io/v1alpha1
kind: Instrumentation
metadata:
  name: my-instrumentation
spec:
  env:
  - name: OTEL_EXPORTER_OTLP_PROTOCOL
    value: http/protobuf
  - name: OTEL_RESOURCE_ATTRIBUTES
    value: sampler.ratio=$(OTEL_TRACES_SAMPLER_ARG),service.instance.id=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAMESPACE)/$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),deployment.environment=$(OTEL_RESOURCE_ATTRIBUTES_DEPLOYMENT_ENVIRONMENT),
  - name: OTEL_KUBERNETES_HOST_IP
    valueFrom:
      fieldRef:
        fieldPath: status.hostIP
  - name: OTEL_RESOURCE_ATTRIBUTES_POD_NAMESPACE
    valueFrom:
      fieldRef:
        fieldPath: metadata.namespace
  - name: OTEL_RESOURCE_ATTRIBUTES_DEPLOYMENT_ENVIRONMENT
    valueFrom:
      fieldRef:
        fieldPath: metadata.labels['environment']
  exporter:
    endpoint: http://$(OTEL_KUBERNETES_HOST_IP):4318
  java:
    env:
    - name: OTEL_LOGS_EXPORTER
      value: otlp
    image:  ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-java:1.28.0
  sampler:
    argument: "1"
    type: parentbased_traceidratio
@erenming erenming added the Feature Request Suggest an idea for this project label Aug 25, 2023
@erenming erenming reopened this Aug 25, 2023
@jack-berg
Copy link
Member

This is a good example. I would have expected the kubernetes environment variable substitution syntax to support setting a fallback / default, something like $(OTEL_RESOURCE_ATTRIBUTES_DEPLOYMENT_ENVIRONMENT:-foo), but it appears that's not the case.

I don't see anything in the environment variable specification or the supplemental notes that says that null / blank / empty entries are unacceptable.

@erenming
Copy link
Author

You're right @jack-berg . And you have a well consideration for environment's default value, this will work fine for more cases include empty string case.
But as for now the offfical specification dont't have any thing about env default. So i think we could allow empty-value-pair just like other languages and avoid this kind of exception.

@laurit
Copy link
Contributor

laurit commented Oct 4, 2023

@jack-berg I think this can be closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Suggest an idea for this project
Projects
None yet
Development

No branches or pull requests

3 participants