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

Add configmap for tracing config #6897

Merged
merged 1 commit into from
Sep 8, 2023
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
84 changes: 2 additions & 82 deletions cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,16 @@ limitations under the License.
package main

import (
"context"
"flag"
"log"
"net/http"
"os"
"time"

"github.com/tektoncd/pipeline/pkg/apis/pipeline"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun"
"github.com/tektoncd/pipeline/pkg/reconciler/resolutionrequest"
"github.com/tektoncd/pipeline/pkg/reconciler/taskrun"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/exporters/jaeger"
"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/sdk/resource"
tracesdk "go.opentelemetry.io/otel/sdk/trace"
semconv "go.opentelemetry.io/otel/semconv/v1.12.0"
"go.opentelemetry.io/otel/trace"
corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/rest"
"k8s.io/utils/clock"
Expand All @@ -49,10 +40,6 @@ import (
const (
// ControllerLogKey is the name of the logger for the controller cmd
ControllerLogKey = "tekton-pipelines-controller"
// TracerProviderPipelineRun is the name of TraceProvider used pipeline reconciler
TracerProviderPipelineRun = "pipeline-reconciler"
// TracerProviderTaskRun is the name of TracerProvider used in taskrun reconciler
TracerProviderTaskRun = "taskrun-reconciler"
)

func main() {
Expand Down Expand Up @@ -110,81 +97,14 @@ func main() {
log.Fatal(http.ListenAndServe(":"+port, mux)) // #nosec G114 -- see https://github.com/securego/gosec#available-rules
}()

// initialize opentelemetry
tpPipelineRun, err := tracerProvider(TracerProviderPipelineRun)
if err != nil {
log.Printf("failed to initialize tracerProvider for pipelinerun, falling back to no-op provider, %s", err.Error())
tpPipelineRun = trace.NewNoopTracerProvider()
}
tpTaskrun, err := tracerProvider(TracerProviderTaskRun)
if err != nil {
log.Printf("failed to initialize tracerProvider for taskrun, falling back to no-op provider, %s", err.Error())
tpTaskrun = trace.NewNoopTracerProvider()
}
otel.SetTextMapPropagator(propagation.TraceContext{})
ctx, cancel := context.WithCancel(ctx)
defer cancel()

ctx = filteredinformerfactory.WithSelectors(ctx, v1beta1.ManagedByLabelKey)
sharedmain.MainWithConfig(ctx, ControllerLogKey, cfg,
taskrun.NewController(opts, clock.RealClock{}, tpTaskrun),
pipelinerun.NewController(opts, clock.RealClock{}, tpPipelineRun),
taskrun.NewController(opts, clock.RealClock{}),
pipelinerun.NewController(opts, clock.RealClock{}),
resolutionrequest.NewController(clock.RealClock{}),
)

// Cleanly shutdown and flush telemetry when the application exits.
defer func(ctx context.Context) {
// Do not make the application hang when it is shutdown.
ctx, cancel = context.WithTimeout(ctx, time.Second*5)
defer cancel()

// shutdown is only needed when tracerProvider is inialized with jaeger
// not needed when tracerProvider is NewNoopTracerProvider
if tp, ok := tpPipelineRun.(*tracesdk.TracerProvider); ok {
if err := tp.Shutdown(ctx); err != nil {
log.Printf("Unable to shutdown tracerProvider for pipelinerun, %s", err.Error())
}
}
if tp, ok := tpTaskrun.(*tracesdk.TracerProvider); ok {
if err := tp.Shutdown(ctx); err != nil {
log.Printf("Unable to shutdown tracerProvider for taskrun, %s", err.Error())
}
}
}(ctx)
}

func handler(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}

// tracerProvider returns an OpenTelemetry TracerProvider configured to use
// the Jaeger exporter that will send spans to the provided url. The returned
// TracerProvider will also use a Resource configured with all the information
// about the application.
func tracerProvider(service string) (trace.TracerProvider, error) {
// Create the Jaeger exporter
// The following env variables are used by the sdk for creating the exporter
// - OTEL_EXPORTER_JAEGER_ENDPOINT is the HTTP endpoint for sending spans directly to a collector.
// - OTEL_EXPORTER_JAEGER_USER is the username to be sent as authentication to the collector endpoint.
// - OTEL_EXPORTER_JAEGER_PASSWORD is the password to be sent as authentication to the collector endpoint.

if _, e := os.LookupEnv("OTEL_EXPORTER_JAEGER_ENDPOINT"); !e {
// jaeger endpoint is not defined, disable tracing and return no-op tracerProvider
return trace.NewNoopTracerProvider(), nil
}

exp, err := jaeger.New(jaeger.WithCollectorEndpoint())
if err != nil {
return nil, err
}
// Initialize tracerProvider with the jaeger exporter
tp := tracesdk.NewTracerProvider(
tracesdk.WithBatcher(exp),
// Record information about the service in a Resource.
tracesdk.WithResource(resource.NewWithAttributes(
semconv.SchemaURL,
semconv.ServiceNameKey.String(service),
)),
)
return tp, nil
}
44 changes: 44 additions & 0 deletions config/config-tracing.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Copyright 2023 The Tekton 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.

apiVersion: v1
kind: ConfigMap
metadata:
name: config-tracing
namespace: tekton-pipelines
labels:
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-pipelines
data:
_example: |
################################
# #
# EXAMPLE CONFIGURATION #
# #
################################
# This block is not actually functional configuration,
# but serves to illustrate the available configuration
# options and document them in a way that is accessible
# to users that `kubectl edit` this config map.
#
# These sample configuration options may be copied out of
# this example block and unindented to be in the data block
# to actually change the configuration.
#
# Enable sending traces to defined endpoint by setting this to true
enabled: "true"
#
# API endpoint to send the traces to
# (optional): The default value is given below
endpoint: "http://jaeger-collector.jaeger.svc.cluster.local:14268/api/traces"
Copy link
Member

Choose a reason for hiding this comment

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

would it make sense to either:

  • assume that if someone has configured an endpoint, that they want tracing enabled, i.e. remove the "enabled" setting
  • make this configuration real configuration instead of an example configuration, and set this to be the actual default endpoint and set enabled to "false" by default
    ?

Copy link
Contributor Author

@kmjayadeep kmjayadeep Sep 8, 2023

Choose a reason for hiding this comment

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

assume that if someone has configured an endpoint, that they want tracing enabled, i.e. remove the "enabled" setting

We will add more config options later. So it makes sense to make the enable flag explicit rather than based on the endpoint, which will be confusing to the users.

make this configuration real configuration instead of an example configuration, and set this to be the actual default endpoint and set enabled to "false" by default

We follow this pattern of giving only an example for other configmaps like config-events and config-observability. It is better to keep it consistent with those.

10 changes: 6 additions & 4 deletions docs/developers/tracing.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ Check the official [Jaeger docs](https://www.jaegertracing.io/docs/) on how to w

## Enabling tracing

Tekton pipelines controller expects the following environment variables to be able to connect to jaeger:
The configmap `config/config-tracing.yaml` contains the configuration for tracing. It contains the following fields:

* enabled: Set this to true to enable tracing
* endpoint: API endpoint for jaeger collector to send the traces. By default the endpoint is configured to be `http://jaeger-collector.jaeger.svc.cluster.local:14268/api/traces`.

Tekton pipelines controller also supports the following additional environment variables to be able to connect to jaeger:

* `OTEL_EXPORTER_JAEGER_ENDPOINT` is the HTTP endpoint for sending spans directly to a collector.
* `OTEL_EXPORTER_JAEGER_USER` is the username to be sent as authentication to the collector endpoint.
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious why some of the configuration is moving to a configmap but not all of it. Is it because the password can be read from a secret when used in an envvar but not when used in a configmap? maybe the configmap could accept the name of a secret where the password is stored?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's the idea. Configmap will have a secret name where creds are stored. It requires a separate logic for reconciling the secret. I wanted to keep the scope of the PR small for easy review process. I already have a WIP follow-up PR for this change in my fork here if you are interested to take a look.

* `OTEL_EXPORTER_JAEGER_PASSWORD` is the password to be sent as authentication to the collector endpoint.

`OTEL_EXPORTER_JAEGER_ENDPOINT` is the only manadatory variable to enable tracing. You can find these variables in the controller manifest as well.
9 changes: 9 additions & 0 deletions pkg/apis/config/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type Config struct {
Metrics *Metrics
SpireConfig *sc.SpireConfig
Events *Events
Tracing *Tracing
}

// FromContext extracts a Config from the provided context.
Expand All @@ -57,6 +58,7 @@ func FromContextOrDefaults(ctx context.Context) *Config {
Metrics: DefaultMetrics.DeepCopy(),
SpireConfig: DefaultSpire.DeepCopy(),
Events: DefaultEvents.DeepCopy(),
Tracing: DefaultTracing.DeepCopy(),
}
}

Expand Down Expand Up @@ -84,6 +86,7 @@ func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value i
GetMetricsConfigName(): NewMetricsFromConfigMap,
GetSpireConfigName(): NewSpireConfigFromConfigMap,
GetEventsConfigName(): NewEventsFromConfigMap,
GetTracingConfigName(): NewTracingFromConfigMap,
},
onAfterStore...,
),
Expand Down Expand Up @@ -111,6 +114,11 @@ func (s *Store) Load() *Config {
if metrics == nil {
metrics = DefaultMetrics.DeepCopy()
}
tracing := s.UntypedLoad(GetTracingConfigName())
if tracing == nil {
tracing = DefaultTracing.DeepCopy()
}

spireconfig := s.UntypedLoad(GetSpireConfigName())
if spireconfig == nil {
spireconfig = DefaultSpire.DeepCopy()
Expand All @@ -124,6 +132,7 @@ func (s *Store) Load() *Config {
Defaults: defaults.(*Defaults).DeepCopy(),
FeatureFlags: featureFlags.(*FeatureFlags).DeepCopy(),
Metrics: metrics.(*Metrics).DeepCopy(),
Tracing: tracing.(*Tracing).DeepCopy(),
SpireConfig: spireconfig.(*sc.SpireConfig).DeepCopy(),
Events: events.(*Events).DeepCopy(),
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/config/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,22 @@ func TestStoreLoadWithContext(t *testing.T) {
metricsConfig := test.ConfigMapFromTestFile(t, "config-observability")
spireConfig := test.ConfigMapFromTestFile(t, "config-spire")
eventsConfig := test.ConfigMapFromTestFile(t, "config-events")
tracingConfig := test.ConfigMapFromTestFile(t, "config-tracing")

expectedDefaults, _ := config.NewDefaultsFromConfigMap(defaultConfig)
expectedFeatures, _ := config.NewFeatureFlagsFromConfigMap(featuresConfig)
metrics, _ := config.NewMetricsFromConfigMap(metricsConfig)
expectedSpireConfig, _ := config.NewSpireConfigFromConfigMap(spireConfig)
expectedEventsConfig, _ := config.NewEventsFromConfigMap(eventsConfig)
expectedTracingConfig, _ := config.NewTracingFromConfigMap(tracingConfig)

expected := &config.Config{
Defaults: expectedDefaults,
FeatureFlags: expectedFeatures,
Metrics: metrics,
SpireConfig: expectedSpireConfig,
Events: expectedEventsConfig,
Tracing: expectedTracingConfig,
}

store := config.NewStore(logtesting.TestLogger(t))
Expand All @@ -54,6 +57,7 @@ func TestStoreLoadWithContext(t *testing.T) {
store.OnConfigChanged(metricsConfig)
store.OnConfigChanged(spireConfig)
store.OnConfigChanged(eventsConfig)
store.OnConfigChanged(tracingConfig)

cfg := config.FromContext(store.ToContext(context.Background()))

Expand All @@ -69,6 +73,7 @@ func TestStoreLoadWithContext_Empty(t *testing.T) {
Metrics: config.DefaultMetrics.DeepCopy(),
SpireConfig: config.DefaultSpire.DeepCopy(),
Events: config.DefaultEvents.DeepCopy(),
Tracing: config.DefaultTracing.DeepCopy(),
}

store := config.NewStore(logtesting.TestLogger(t))
Expand Down
29 changes: 29 additions & 0 deletions pkg/apis/config/testdata/config-tracing-empty.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Copyright 2022 The Tekton 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.

apiVersion: v1
kind: ConfigMap
metadata:
name: config-tracing
namespace: tekton-pipelines
labels:
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-pipelines
data:
_example: |
################################
# #
# EXAMPLE CONFIGURATION #
# #
################################
25 changes: 25 additions & 0 deletions pkg/apis/config/testdata/config-tracing-enabled.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Copyright 2022 The Tekton 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.

apiVersion: v1
kind: ConfigMap
metadata:
name: config-tracing
namespace: tekton-pipelines
labels:
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-pipelines
data:
enabled: true
endpoint: http://jaeger-test
24 changes: 24 additions & 0 deletions pkg/apis/config/testdata/config-tracing.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Copyright 2022 The Tekton 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.

apiVersion: v1
kind: ConfigMap
metadata:
name: config-tracing
namespace: tekton-pipelines
labels:
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-pipelines
data:
enabled: false
Loading