Skip to content

Commit

Permalink
[connector/servicegraph] Update virtual node defaults and change feat…
Browse files Browse the repository at this point in the history
…ure gate to beta (#31735)

**Description:**

- Change default values to peer.service, db.name and db.system (in this
order). These are compliant with conventions, and the same as the tempo
service graph uses.
- Switch the feature from "alpha" to "beta". Virtual nodes are working
well on the tempo side, and feature could be disabled anyway by using an
empty list if needed.
- Reword documentation for improved clarity
- Fix "processor" that should be "connector" instead since recent commit

**Link to tracking Issue:**
#31734

**Testing:** No functional change requiring additional tests

**Documentation:** Updated documentation
  • Loading branch information
ogxd authored Apr 14, 2024
1 parent f107dd4 commit 1259b63
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 9 deletions.
20 changes: 20 additions & 0 deletions .chloggen/servicegraphconnector-virtualnodes.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: connector/servicegraphconnector

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Change `connector.servicegraph.virtualNode` feature gate from Alpha to Beta (now enabled by default) and change `virtual_node_peer_attributes` default values.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [31734]

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [user]
6 changes: 3 additions & 3 deletions connector/servicegraphconnector/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ datasources:
The following settings are required:
- `metrics_exporter`: the name of the exporter that this processor will write metrics to. This exporter **must** be present in a pipeline.
- `metrics_exporter`: the name of the exporter that this connector will write metrics to. This exporter **must** be present in a pipeline.
- `latency_histogram_buckets`: the list of durations defining the latency histogram buckets.
- Default: `[2ms, 4ms, 6ms, 8ms, 10ms, 50ms, 100ms, 200ms, 400ms, 800ms, 1s, 1400ms, 2s, 5s, 10s, 15s]`
- `dimensions`: the list of dimensions to add together with the default dimensions defined above.
Expand All @@ -134,8 +134,8 @@ The following settings can be optionally configured:
- Default: `1m`
- `store_expiration_loop`: the time to expire old entries from the store periodically.
- Default: `2s`
- `virtual_node_peer_attributes`: the list of attributes need to match for building virtual server node, the higher the front, the higher the priority.
- Default: `[db.name, net.sock.peer.addr, net.peer.name, rpc.service, net.sock.peer.name, net.peer.name, http.url, http.target]`
- `virtual_node_peer_attributes`: the list of attributes, ordered by priority, whose presence in a client span will result in the creation of a virtual server node. An empty list disables virtual node creation.
- Default: `[peer.service, db.name, db.system]`
- `metrics_flush_interval`: the interval at which metrics are flushed to the exporter.
- Default: Metrics are flushed on every received batch of traces.
- `database_name_attribute`: the attribute name used to identify the database name from span attributes.
Expand Down
4 changes: 2 additions & 2 deletions connector/servicegraphconnector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ var (
}

defaultPeerAttributes = []string{
semconv.AttributeDBName, semconv.AttributeNetSockPeerAddr, semconv.AttributeNetPeerName, semconv.AttributeRPCService, semconv.AttributeNetSockPeerName, semconv.AttributeNetPeerName, semconv.AttributeHTTPURL, semconv.AttributeHTTPTarget,
semconv.AttributePeerService, semconv.AttributeDBName, semconv.AttributeDBSystem,
}

defaultDatabaseNameAttribute = semconv.AttributeDBName
Expand Down Expand Up @@ -380,7 +380,7 @@ func (p *serviceGraphConnector) onExpire(e *store.Edge) {

p.statExpiredEdges.Add(context.Background(), 1)

if virtualNodeFeatureGate.IsEnabled() {
if virtualNodeFeatureGate.IsEnabled() && len(p.config.VirtualNodePeerAttributes) > 0 {
e.ConnectionType = store.VirtualNode
if len(e.ClientService) == 0 && e.Key.SpanIDIsEmpty() {
e.ClientService = "user"
Expand Down
8 changes: 4 additions & 4 deletions connector/servicegraphconnector/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,21 @@ var virtualNodeFeatureGate, legacyMetricNamesFeatureGate, legacyLatencyUnitMsFea
func init() {
virtualNodeFeatureGate = featuregate.GlobalRegistry().MustRegister(
virtualNodeFeatureGateID,
featuregate.StageAlpha,
featuregate.WithRegisterDescription("When enabled, when the edge expires, processor checks if it has peer attributes(`db.name, net.sock.peer.addr, net.peer.name, rpc.service, http.url, http.target`), and then aggregate the metrics with virtual node."),
featuregate.StageBeta,
featuregate.WithRegisterDescription("When enabled and setting `virtual_node_peer_attributes` is not empty, the connector looks for the presence of these attributes in span to create virtual server nodes."),
featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/17196"),
)
// TODO: Remove this feature gate when the legacy metric names are removed.
legacyMetricNamesFeatureGate = featuregate.GlobalRegistry().MustRegister(
legacyLatencyMetricNamesFeatureGateID,
featuregate.StageAlpha, // Alpha because we want it disabled by default.
featuregate.WithRegisterDescription("When enabled, processor uses legacy latency metric names."),
featuregate.WithRegisterDescription("When enabled, connector uses legacy latency metric names."),
featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/18743,https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/16578"),
)
legacyLatencyUnitMsFeatureGate = featuregate.GlobalRegistry().MustRegister(
legacyLatencyUnitMs,
featuregate.StageAlpha, // Alpha because we want it disabled by default.
featuregate.WithRegisterDescription("When enabled, processor reports latency in milliseconds, instead of seconds."),
featuregate.WithRegisterDescription("When enabled, connector reports latency in milliseconds, instead of seconds."),
featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/27488"),
)
}
Expand Down

0 comments on commit 1259b63

Please sign in to comment.