hanahmily commented on code in PR #1206: URL: https://github.com/apache/skywalking-banyandb/pull/1206#discussion_r3535492047
########## banyand/trace/plugin_telemetry.go: ########## @@ -0,0 +1,712 @@ +// Licensed to Apache Software Foundation (ASF) under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Apache Software Foundation (ASF) licenses this file to you 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 +// +// http://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. + +package trace + +import ( + "fmt" + "slices" + "strings" + "sync" + "time" + "unicode" + + "github.com/rs/zerolog" + + "github.com/apache/skywalking-banyandb/banyand/observability" + "github.com/apache/skywalking-banyandb/pkg/logger" + "github.com/apache/skywalking-banyandb/pkg/meter" + "github.com/apache/skywalking-banyandb/pkg/pipeline/sdk" +) + +const ( + // maxSeriesPerInstrument is the cardinality cap for custom label-value tuples + // per instrument instance. + maxSeriesPerInstrument = 100 + // maxMetricNameLen is the maximum sanitized metric name length. + maxMetricNameLen = 64 + // logRatePerSec is the sustained token-bucket refill rate (tokens/second). + logRatePerSec = 50.0 + // logBurstSize is the maximum burst capacity of the log token bucket. + logBurstSize = 100.0 + // maxMsgLen is the maximum log message length in bytes. + maxMsgLen = 1024 + // maxLogFields is the maximum number of structured key/value pairs emitted. + maxLogFields = 16 + // suppressWarnInterval is the minimum interval between rate-limiter summary WARNs. + suppressWarnInterval = 5 * time.Second + // overflowSentinel is the fixed label value substituted when cardinality is exceeded. + overflowSentinel = "__overflow__" +) + +// reservedLabelSet contains label names plugins may not declare; the host +// always prepends group and plugin_name itself. +var reservedLabelSet = map[string]struct{}{ + "group": {}, + "plugin_name": {}, +} + +// noopCounter is a zero-allocation sdk.Counter for the bypass path. +type noopCounter struct{} + +func (noopCounter) Inc(_ float64, _ ...string) {} + +// noopGauge is a zero-allocation sdk.Gauge for the bypass path. +type noopGauge struct{} + +func (noopGauge) Set(_ float64, _ ...string) {} +func (noopGauge) Add(_ float64, _ ...string) {} + +// noopHistogram is a zero-allocation sdk.Histogram for the bypass path. +type noopHistogram struct{} + +func (noopHistogram) Observe(_ float64, _ ...string) {} + +// noopMeter is a zero-allocation sdk.Meter for the bypass path. +type noopMeter struct{} + +func (noopMeter) Counter(_ string, _ ...string) sdk.Counter { return noopCounter{} } +func (noopMeter) Gauge(_ string, _ ...string) sdk.Gauge { return noopGauge{} } +func (noopMeter) Histogram(_ string, _ []float64, _ ...string) sdk.Histogram { return noopHistogram{} } + +// instrumentKey identifies a registered instrument in the cache by its full +// metric name and joined label names. +type instrumentKey struct { + fullName string + labelNames string // "\x00"-joined +} + +// boundInstrumentKind distinguishes the three instrument kinds. +type boundInstrumentKind uint8 + +const ( + kindCounter boundInstrumentKind = iota + kindGauge boundInstrumentKind = iota + kindHistogram boundInstrumentKind = iota +) Review Comment: Thanks, but this one is a false positive. In Go, `iota` is the index of the ConstSpec within its `const` block and increments per line **regardless of whether `= iota` is written explicitly on each line**. So these evaluate to distinct values: ```go const ( kindCounter boundInstrumentKind = iota // 0 kindGauge boundInstrumentKind = iota // 1 kindHistogram boundInstrumentKind = iota // 2 ) ``` Kind discrimination therefore works correctly — the teardown switch and the existing telemetry tests (which assert per-kind behavior) pass, which they wouldn't if all three collided. No change needed here. The other three comments are valid and addressed in a follow-up commit. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
