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

[💡 FEATURE REQUEST]: Use Opentelemerty specific environment variables #1848

Closed
fasdalf opened this issue Feb 1, 2024 · 5 comments · Fixed by roadrunner-server/otel#60
Closed
Assignees
Labels
C-enhancement Category: enhancement. Meaning improvements of current module, transport, etc.. C-feature-accepted Category: Feature discussed and accepted P-other Plugin: Other
Milestone

Comments

@fasdalf
Copy link

fasdalf commented Feb 1, 2024

Plugin

GRPC

I have an idea!

Hello! Opentelemerty plugin reads config from .rr.yaml as other plugins do.
Also specification defines environment variables https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/ and https://opentelemetry.io/docs/specs/otel/protocol/exporter/ to read default opentelemetry values from.
Roadrunner 2023 already uses library https://pkg.go.dev/go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc#pkg-overview with support of this variables but over writes them with hardcoded defaults:
c.Resource.ServiceNamespaceKey = fmt.Sprintf("RoadRunner-%s", uuid.NewString())
sdktrace.WithResource(newResource(p.cfg.Resource, cfg.RRVersion())),

Could you restore use of this environment variables as defaults if no values are set in .rr.yaml and use hardcoded strings only when both configurations are empty?

Variables tested to be effective with otel v.1.21.1

OTEL_SERVICE_NAME: "some-micro-service"
OTEL_RESOURCE_ATTRIBUTES:"service.namespace=deployment-namespace,host.name=server-hostname"
OTEL_TRACES_SAMPLER: "traceidratio"
OTEL_TRACES_SAMPLER_ARG: "0.5"
OTEL_TRACES_EXPORTER: "otlp"
OTEL_EXPORTER_OTLP_PROTOCOL: "http"
OTEL_EXPORTER_OTLP_INSECURE: "true"
OTEL_EXPORTER_OTLP_ENDPOINT: "http://172.17.0.1:4318"

Here is an example of code using these variables with InitFromEnvironment()

package trace

// Based on
// https://opentelemetry.io/docs/instrumentation/go/manual/
// https://github.com/open-telemetry/opentelemetry-go-contrib/blob/v1.21.1/instrumentation/google.golang.org/grpc/otelgrpc/example/server/main.go#L54

import (
	"context"
	env "github.com/caarlos0/env/v6"
	log "github.com/sirupsen/logrus"
	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
	"go.opentelemetry.io/otel"
	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp"
	sdktrace "go.opentelemetry.io/otel/sdk/trace"
	"go.opentelemetry.io/otel/trace"
	"google.golang.org/grpc/stats"
)

type TraceConfig struct {
	ServiceName                string `env:"OTEL_SERVICE_NAME" envDefault:"your-service-name"`
}

var Config TraceConfig
var Tracer trace.Tracer
var TracerProvider *sdktrace.TracerProvider

func InitFromEnvironment() (*sdktrace.TracerProvider, error) {
	config := TraceConfig{}
	if err := env.Parse(&config); err != nil {
		log.Fatalln(err)
	}

	return InitFromConfig(config)
}

func InitFromConfig(config TraceConfig) (*sdktrace.TracerProvider, error) {
	Config = config
	Tracer = otel.Tracer(Config.ServiceName)
	return InitTracerProvider()
}

// InitTracerProvider configures an OpenTelemetry exporter and trace provider.
func InitTracerProvider() (*sdktrace.TracerProvider, error) {
	// Set up exporter
	exporter, err := otlptracehttp.New(
		context.Background(),
	)
	
	if err != nil {
		return nil, err
	}

	tp := sdktrace.NewTracerProvider(
		sdktrace.WithBatcher(exporter),
	)
	otel.SetTracerProvider(tp)
	TracerProvider = tp
	return tp, nil
}

// [Unrelated code removed.]
@fasdalf fasdalf added the C-feature-request Category: feature requested, but need to be discussed label Feb 1, 2024
@rustatian
Copy link
Member

Hey @fasdalf 👋
Fair take, I was waiting for a ticket like this 😃
Sure, OTEL specific variables might be defined via env variables and this FR is approved by default 👍
Let me think of how to implement this w/o extra third-party libraries (I mean env/v6).

@rustatian rustatian removed the C-feature-request Category: feature requested, but need to be discussed label Feb 1, 2024
@rustatian rustatian assigned rustatian and unassigned rustatian Feb 1, 2024
@rustatian rustatian added C-enhancement Category: enhancement. Meaning improvements of current module, transport, etc.. C-feature-accepted Category: Feature discussed and accepted P-other Plugin: Other labels Feb 1, 2024
@rustatian rustatian added this to the v2023.3.11 milestone Feb 1, 2024
@rustatian
Copy link
Member

Hey @fasdalf 👋
Remember, that you may use the following well-known syntax, e.g.:

otel:
    # other options
    endpoint: ${OTEL_EXPORTER_OTLP_ENDPOINT:-http://collector:4318}

In this case, if the OTEL_EXPORTER_OTLP_ENDPOINT env variable is not set, default http://collector:4318 value for the endpoint would be used.

@fasdalf
Copy link
Author

fasdalf commented Feb 8, 2024

otel:
    # other options
    endpoint: ${OTEL_EXPORTER_OTLP_ENDPOINT:-http://collector:4318}

Thank you, @rustatian! I gave it a try and it worked!
Is it suitable to add this variables, especially OTEL_SERVICE_NAME, to config examples shipped with the Roadrunner?

@rustatian
Copy link
Member

rustatian commented Feb 8, 2024

Cool 🥳
Yeah, I also forgot that RR supports Bash parameter expansion syntax. It would be cool if you document your configuration experience here: link 🙏

@arku31
Copy link

arku31 commented Feb 12, 2024

Hey, I've finalized the configuration and noticed one thing that sounds more like an issue.

if I am passing this variable to Roadrunner (e.g. via docker-compose)
OTEL_EXPORTER_OTLP_ENDPOINT: http://collector:4318

It is working with PHP OTEL like a charm.
Whenever I enable otel for RR (with GRPC, not sure if http reproducible), it starts to complain

 | 2024/02/12 15:01:27 traces export: parse "http://http:%2F%2Fcollector:4318/v1/traces": invalid URL escape "%2F"

notice that double "http://".

If I do not add http:// in URL => PHP version doesn't work as it tries to connect to https (default)

This leads to the situation that the hostname has to be configured without http:// in the configuration, like this:
OTEL_EXPORTER_OTLP_ENDPOINT: collector:4318

and on the PHP side I had to do some dirty trick

$otelHost = 'http://'.getenv('OTEL_EXPORTER_OTLP_ENDPOINT');
putenv("OTEL_EXPORTER_OTLP_ENDPOINT=$otelHost");

Besides that, works like a charm with collector/zipkin and/or datadog 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: enhancement. Meaning improvements of current module, transport, etc.. C-feature-accepted Category: Feature discussed and accepted P-other Plugin: Other
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants