Skip to content

Commit

Permalink
Support tracing via the ObservabilityPolicy (#2004)
Browse files Browse the repository at this point in the history
* Support tracing via the ObservabilityPolicy

Problem: As a user, I want to be able to enable tracing for requests flowing through NGF to my backend applications, so I can understand and debug my requests.

Solution: Using the ObservabilityPolicy, an app dev can now enable and configure tracing for their Route(s). The policy will only be applied if the NginxProxy configuration containing the tracing collector endpoint has been defined and attached to the GatewayClass. The policy can be attached to one or more HTTP or GRPC Routes.

Updated span attributes from the NginxProxy to be applied at the location block context, otherwise they are overwritten.

Also added functional tests to ensure that the complete solution is working.

* Add functional tests for tracing

Verify end to end tracing by creating a collector and sending trace data to it.

Test cases:
1. one policy attached to one route
2. one policy attached to multiple routes
  • Loading branch information
sjberman authored May 29, 2024
1 parent da36700 commit 4859cca
Show file tree
Hide file tree
Showing 71 changed files with 2,802 additions and 845 deletions.
13 changes: 9 additions & 4 deletions apis/v1alpha1/observabilitypolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,15 @@ type ObservabilityPolicySpec struct {
// +optional
Tracing *Tracing `json:"tracing,omitempty"`

// TargetRef identifies an API object to apply the policy to.
// Object must be in the same namespace as the policy.
//
// TargetRefs identifies the API object(s) to apply the policy to.
// Objects must be in the same namespace as the policy.
// Support: HTTPRoute
TargetRef gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRef"`
//
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:validation:XValidation:message="TargetRef Kind must be: HTTPRoute or GRPCRoute",rule="(self.exists(t, t.kind=='HTTPRoute') || self.exists(t, t.kind=='GRPCRoute'))"
// +kubebuilder:validation:XValidation:message="TargetRef Group must be gateway.networking.k8s.io.",rule="self.all(t, t.group=='gateway.networking.k8s.io')"
//nolint:lll
TargetRefs []gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRefs"`
}

// Tracing allows for enabling and configuring OpenTelemetry tracing.
Expand All @@ -60,6 +64,7 @@ type Tracing struct {

// Ratio is the percentage of traffic that should be sampled. Integer from 0 to 100.
// By default, 100% of http requests are traced. Not applicable for parent-based tracing.
// If ratio is set to 0, tracing is disabled.
//
// +optional
// +kubebuilder:validation:Minimum=0
Expand Down
16 changes: 14 additions & 2 deletions apis/v1alpha1/policy_methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
// Figure out a way to generate these methods for all our policies.
// These methods implement the policies.Policy interface which extends client.Object to add the following methods.

func (p *ClientSettingsPolicy) GetTargetRef() v1alpha2.LocalPolicyTargetReference {
return p.Spec.TargetRef
func (p *ClientSettingsPolicy) GetTargetRefs() []v1alpha2.LocalPolicyTargetReference {
return []v1alpha2.LocalPolicyTargetReference{p.Spec.TargetRef}
}

func (p *ClientSettingsPolicy) GetPolicyStatus() v1alpha2.PolicyStatus {
Expand All @@ -19,3 +19,15 @@ func (p *ClientSettingsPolicy) GetPolicyStatus() v1alpha2.PolicyStatus {
func (p *ClientSettingsPolicy) SetPolicyStatus(status v1alpha2.PolicyStatus) {
p.Status = status
}

func (p *ObservabilityPolicy) GetTargetRefs() []v1alpha2.LocalPolicyTargetReference {
return p.Spec.TargetRefs
}

func (p *ObservabilityPolicy) GetPolicyStatus() v1alpha2.PolicyStatus {
return p.Status
}

func (p *ObservabilityPolicy) SetPolicyStatus(status v1alpha2.PolicyStatus) {
p.Status = status
}
7 changes: 6 additions & 1 deletion apis/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions charts/nginx-gateway-fabric/templates/rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ rules:
resources:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
verbs:
- list
- watch
Expand All @@ -130,6 +131,7 @@ rules:
resources:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
verbs:
- update
{{- if .Values.nginxGateway.leaderElection.enable }}
Expand Down
69 changes: 41 additions & 28 deletions config/crd/bases/gateway.nginx.org_observabilitypolicies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,35 +50,47 @@ spec:
spec:
description: Spec defines the desired state of the ObservabilityPolicy.
properties:
targetRef:
targetRefs:
description: |-
TargetRef identifies an API object to apply the policy to.
Object must be in the same namespace as the policy.
TargetRefs identifies the API object(s) to apply the policy to.
Objects must be in the same namespace as the policy.
Support: HTTPRoute
properties:
group:
description: Group is the group of the target resource.
maxLength: 253
pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
kind:
description: Kind is kind of the target resource.
maxLength: 63
minLength: 1
pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$
type: string
name:
description: Name is the name of the target resource.
maxLength: 253
minLength: 1
type: string
required:
- group
- kind
- name
type: object
items:
description: |-
LocalPolicyTargetReference identifies an API object to apply a direct or
inherited policy to. This should be used as part of Policy resources
that can target Gateway API resources. For more information on how this
policy attachment model works, and a sample Policy resource, refer to
the policy attachment documentation for Gateway API.
properties:
group:
description: Group is the group of the target resource.
maxLength: 253
pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
kind:
description: Kind is kind of the target resource.
maxLength: 63
minLength: 1
pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$
type: string
name:
description: Name is the name of the target resource.
maxLength: 253
minLength: 1
type: string
required:
- group
- kind
- name
type: object
maxItems: 16
type: array
x-kubernetes-validations:
- message: 'TargetRef Kind must be: HTTPRoute or GRPCRoute'
rule: (self.exists(t, t.kind=='HTTPRoute') || self.exists(t, t.kind=='GRPCRoute'))
- message: TargetRef Group must be gateway.networking.k8s.io.
rule: self.all(t, t.group=='gateway.networking.k8s.io')
tracing:
description: Tracing allows for enabling and configuring tracing.
properties:
Expand All @@ -96,6 +108,7 @@ spec:
description: |-
Ratio is the percentage of traffic that should be sampled. Integer from 0 to 100.
By default, 100% of http requests are traced. Not applicable for parent-based tracing.
If ratio is set to 0, tracing is disabled.
format: int32
maximum: 100
minimum: 0
Expand Down Expand Up @@ -155,7 +168,7 @@ spec:
- message: ratio can only be specified if strategy is of type ratio
rule: '!(has(self.ratio) && self.strategy != ''ratio'')'
required:
- targetRef
- targetRefs
type: object
status:
description: Status defines the state of the ObservabilityPolicy.
Expand Down
69 changes: 41 additions & 28 deletions deploy/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -832,35 +832,47 @@ spec:
spec:
description: Spec defines the desired state of the ObservabilityPolicy.
properties:
targetRef:
targetRefs:
description: |-
TargetRef identifies an API object to apply the policy to.
Object must be in the same namespace as the policy.
TargetRefs identifies the API object(s) to apply the policy to.
Objects must be in the same namespace as the policy.
Support: HTTPRoute
properties:
group:
description: Group is the group of the target resource.
maxLength: 253
pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
kind:
description: Kind is kind of the target resource.
maxLength: 63
minLength: 1
pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$
type: string
name:
description: Name is the name of the target resource.
maxLength: 253
minLength: 1
type: string
required:
- group
- kind
- name
type: object
items:
description: |-
LocalPolicyTargetReference identifies an API object to apply a direct or
inherited policy to. This should be used as part of Policy resources
that can target Gateway API resources. For more information on how this
policy attachment model works, and a sample Policy resource, refer to
the policy attachment documentation for Gateway API.
properties:
group:
description: Group is the group of the target resource.
maxLength: 253
pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
kind:
description: Kind is kind of the target resource.
maxLength: 63
minLength: 1
pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$
type: string
name:
description: Name is the name of the target resource.
maxLength: 253
minLength: 1
type: string
required:
- group
- kind
- name
type: object
maxItems: 16
type: array
x-kubernetes-validations:
- message: 'TargetRef Kind must be: HTTPRoute or GRPCRoute'
rule: (self.exists(t, t.kind=='HTTPRoute') || self.exists(t, t.kind=='GRPCRoute'))
- message: TargetRef Group must be gateway.networking.k8s.io.
rule: self.all(t, t.group=='gateway.networking.k8s.io')
tracing:
description: Tracing allows for enabling and configuring tracing.
properties:
Expand All @@ -878,6 +890,7 @@ spec:
description: |-
Ratio is the percentage of traffic that should be sampled. Integer from 0 to 100.
By default, 100% of http requests are traced. Not applicable for parent-based tracing.
If ratio is set to 0, tracing is disabled.
format: int32
maximum: 100
minimum: 0
Expand Down Expand Up @@ -937,7 +950,7 @@ spec:
- message: ratio can only be specified if strategy is of type ratio
rule: '!(has(self.ratio) && self.strategy != ''ratio'')'
required:
- targetRef
- targetRefs
type: object
status:
description: Status defines the state of the ObservabilityPolicy.
Expand Down
2 changes: 2 additions & 0 deletions deploy/manifests/nginx-gateway-experimental.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ rules:
resources:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
verbs:
- list
- watch
Expand All @@ -112,6 +113,7 @@ rules:
resources:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
verbs:
- update
- apiGroups:
Expand Down
2 changes: 2 additions & 0 deletions deploy/manifests/nginx-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ rules:
resources:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
verbs:
- list
- watch
Expand All @@ -109,6 +110,7 @@ rules:
resources:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
verbs:
- update
- apiGroups:
Expand Down
2 changes: 2 additions & 0 deletions deploy/manifests/nginx-plus-gateway-experimental.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ rules:
resources:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
verbs:
- list
- watch
Expand All @@ -118,6 +119,7 @@ rules:
resources:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
verbs:
- update
- apiGroups:
Expand Down
2 changes: 2 additions & 0 deletions deploy/manifests/nginx-plus-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ rules:
resources:
- nginxproxies
- clientsettingspolicies
- observabilitypolicies
verbs:
- list
- watch
Expand All @@ -115,6 +116,7 @@ rules:
resources:
- nginxgateways/status
- clientsettingspolicies/status
- observabilitypolicies/status
verbs:
- update
- apiGroups:
Expand Down
12 changes: 6 additions & 6 deletions docs/proposals/observability.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Enhancement Proposal-1778: Observability Policy

- Issue: https://github.com/nginxinc/nginx-gateway-fabric/issues/1778
- Status: Implementable
- Status: Completed

## Summary

Expand Down Expand Up @@ -65,10 +65,10 @@ type ObservabilityPolicy struct {
}

type ObservabilityPolicySpec struct {
// TargetRef identifies an API object to apply the policy to.
// Object must be in the same namespace as the policy.
// TargetRefs identifies API object(s) to apply the policy to.
// Objects must be in the same namespace as the policy.
// Support: HTTPRoute
TargetRef gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRef"`
TargetRefs []gatewayv1alpha2.LocalPolicyTargetReference `json:"targetRefs"`

// Tracing allows for enabling and configuring tracing.
//
Expand Down Expand Up @@ -154,8 +154,8 @@ metadata:
name: example-observability-policy
namespace: default
spec:
targetRef:
group: gateway.networking.k8s.io
targetRefs:
- group: gateway.networking.k8s.io
kind: HTTPRoute
name: example-route
tracing:
Expand Down
2 changes: 2 additions & 0 deletions internal/framework/kinds/kinds.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ const (
const (
// ClientSettingsPolicy is the ClientSettingsPolicy kind.
ClientSettingsPolicy = "ClientSettingsPolicy"
// ObservabilityPolicy is the ObservabilityPolicy kind.
ObservabilityPolicy = "ObservabilityPolicy"
// NginxProxy is the NginxProxy kind.
NginxProxy = "NginxProxy"
)
Expand Down
Loading

0 comments on commit 4859cca

Please sign in to comment.