-
Notifications
You must be signed in to change notification settings - Fork 3.9k
otel: subchannel metrics #12202
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
base: master
Are you sure you want to change the base?
otel: subchannel metrics #12202
Changes from all commits
a06568c
daf901b
735665b
a664b2f
2ce087e
2293e7f
623c0f9
c713561
ca8e9ed
b45a907
8309102
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
/* | ||
* Copyright 2025 The gRPC 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 | ||
* | ||
* 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 io.grpc; | ||
|
||
import java.util.List; | ||
|
||
/** | ||
* Represents a long-valued up down counter metric instrument. | ||
*/ | ||
@Internal | ||
public final class LongUpDownCounterMetricInstrument extends PartialMetricInstrument { | ||
public LongUpDownCounterMetricInstrument(int index, String name, String description, String unit, | ||
List<String> requiredLabelKeys, | ||
List<String> optionalLabelKeys, | ||
boolean enableByDefault) { | ||
super(index, name, description, unit, requiredLabelKeys, optionalLabelKeys, enableByDefault); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,6 +144,47 @@ public LongCounterMetricInstrument registerLongCounter(String name, | |
} | ||
} | ||
|
||
/** | ||
* Registers a new Long Up Down Counter metric instrument. | ||
* | ||
* @param name the name of the metric | ||
* @param description a description of the metric | ||
* @param unit the unit of measurement for the metric | ||
* @param requiredLabelKeys a list of required label keys | ||
* @param optionalLabelKeys a list of optional label keys | ||
* @param enableByDefault whether the metric should be enabled by default | ||
* @return the newly created LongUpDownCounterMetricInstrument | ||
* @throws IllegalStateException if a metric with the same name already exists | ||
*/ | ||
public LongUpDownCounterMetricInstrument registerLongUpDownCounter(String name, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add unit test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added for the one above |
||
String description, | ||
String unit, | ||
List<String> requiredLabelKeys, | ||
List<String> optionalLabelKeys, | ||
boolean enableByDefault) { | ||
checkArgument(!Strings.isNullOrEmpty(name), "missing metric name"); | ||
checkNotNull(description, "description"); | ||
checkNotNull(unit, "unit"); | ||
checkNotNull(requiredLabelKeys, "requiredLabelKeys"); | ||
checkNotNull(optionalLabelKeys, "optionalLabelKeys"); | ||
synchronized (lock) { | ||
if (registeredMetricNames.contains(name)) { | ||
throw new IllegalStateException("Metric with name " + name + " already exists"); | ||
} | ||
int index = nextAvailableMetricIndex; | ||
if (index + 1 == metricInstruments.length) { | ||
resizeMetricInstruments(); | ||
} | ||
LongUpDownCounterMetricInstrument instrument = new LongUpDownCounterMetricInstrument( | ||
index, name, description, unit, requiredLabelKeys, optionalLabelKeys, | ||
enableByDefault); | ||
metricInstruments[index] = instrument; | ||
registeredMetricNames.add(name); | ||
nextAvailableMetricIndex += 1; | ||
return instrument; | ||
} | ||
} | ||
|
||
/** | ||
* Registers a new Double Histogram metric instrument. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -415,7 +415,7 @@ void exitIdleMode() { | |
LbHelperImpl lbHelper = new LbHelperImpl(); | ||
lbHelper.lb = loadBalancerFactory.newLoadBalancer(lbHelper); | ||
// Delay setting lbHelper until fully initialized, since loadBalancerFactory is user code and | ||
// may throw. We don't want to confuse our state, even if we will enter panic mode. | ||
// may throw. We don't want to confuse our state, even if we enter panic mode. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the comment is more accurate as it is. After entering panic mode there is nothing much we can do about state, the comment implies delaying entering that panic mode and maintaining a sane state for the channel for as long as possible before bringing in potential user code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a grammar change... |
||
this.lbHelper = lbHelper; | ||
|
||
channelStateManager.gotoState(CONNECTING); | ||
|
@@ -1464,7 +1464,9 @@ void onStateChange(InternalSubchannel is, ConnectivityStateInfo newState) { | |
subchannelTracer, | ||
subchannelLogId, | ||
subchannelLogger, | ||
transportFilters); | ||
transportFilters, | ||
target, | ||
lbHelper.getMetricRecorder()); | ||
oobChannelTracer.reportEvent(new ChannelTrace.Event.Builder() | ||
.setDescription("Child Subchannel created") | ||
.setSeverity(ChannelTrace.Event.Severity.CT_INFO) | ||
|
@@ -1895,7 +1897,8 @@ void onNotInUse(InternalSubchannel is) { | |
subchannelTracer, | ||
subchannelLogId, | ||
subchannelLogger, | ||
transportFilters); | ||
transportFilters, target, | ||
lbHelper.getMetricRecorder()); | ||
|
||
channelTracer.reportEvent(new ChannelTrace.Event.Builder() | ||
.setDescription("Child Subchannel started") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ | |
import io.grpc.LongCounterMetricInstrument; | ||
import io.grpc.LongGaugeMetricInstrument; | ||
import io.grpc.LongHistogramMetricInstrument; | ||
import io.grpc.LongUpDownCounterMetricInstrument; | ||
import io.grpc.MetricInstrument; | ||
import io.grpc.MetricInstrumentRegistry; | ||
import io.grpc.MetricRecorder; | ||
|
@@ -82,7 +83,7 @@ public void addDoubleCounter(DoubleCounterMetricInstrument metricInstrument, dou | |
* Records a long counter value. | ||
* | ||
* @param metricInstrument the {@link LongCounterMetricInstrument} to record. | ||
* @param value the value to record. | ||
* @param value the value to record. Must be non-negative. | ||
* @param requiredLabelValues the required label values for the metric. | ||
* @param optionalLabelValues the optional label values for the metric. | ||
*/ | ||
|
@@ -103,6 +104,32 @@ public void addLongCounter(LongCounterMetricInstrument metricInstrument, long va | |
} | ||
} | ||
|
||
/** | ||
* Adds a long up down counter value. | ||
* | ||
* @param metricInstrument the {@link io.grpc.LongUpDownCounterMetricInstrument} to record. | ||
* @param value the value to record. May be positive, negative or zero. | ||
* @param requiredLabelValues the required label values for the metric. | ||
* @param optionalLabelValues the optional label values for the metric. | ||
*/ | ||
@Override | ||
public void addLongUpDownCounter(LongUpDownCounterMetricInstrument metricInstrument, long value, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add unit tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are just wrappers on the underlying OTel API for UpDownCounter... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but I do see logic here and there are still things to check. See that we have tests in MetricRecorderImplTest for addLongCounter(), which is very similar. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
List<String> requiredLabelValues, | ||
List<String> optionalLabelValues) { | ||
MetricRecorder.super.addLongUpDownCounter(metricInstrument, value, requiredLabelValues, | ||
optionalLabelValues); | ||
for (MetricSink sink : metricSinks) { | ||
int measuresSize = sink.getMeasuresSize(); | ||
if (measuresSize <= metricInstrument.getIndex()) { | ||
// Measures may need updating in two cases: | ||
// 1. When the sink is initially created with an empty list of measures. | ||
// 2. When new metric instruments are registered, requiring the sink to accommodate them. | ||
sink.updateMeasures(registry.getMetricInstruments()); | ||
} | ||
sink.addLongUpDownCounter(metricInstrument, value, requiredLabelValues, optionalLabelValues); | ||
} | ||
} | ||
|
||
/** | ||
* Records a double histogram value. | ||
* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The string here is just for debugging. We typically try to match the API where the value is accessible. See ATTR_AUTHORITY_OVERRIDE just above, for example.