Should we use bit fields to flag data point properties?

[victlu]

We need to flag a few properties in OTLP protocol. Currently, we have a bool for monotonic.

Should we use a 32-bit word (as a bit flag) now to allow future growth?

Concerns:

• Performance of handling a bit-field vs individual bools.
• Backward and forward compatibility.

[victlu]

Initial results on DotNet for Summary kind.

• Generating Summary with Field cost more cycles and is also 9% larger in byte size.

Benchmark results

BenchmarkDotNet=v0.13.0, OS=Windows 10.0.19043.1023 (21H1/May2021Update)
Intel Core i7-1065G7 CPU 1.30GHz, 1 CPU, 8 logical and 4 physical cores
.NET SDK=5.0.202
  [Host]     : .NET 5.0.5 (5.0.521.16609), X64 RyuJIT
  DefaultJob : .NET 5.0.5 (5.0.521.16609), X64 RyuJIT


|         Method | Version | ConfigSet |     Mean |    Error |   StdDev | MessageSize |
|--------------- |-------- |---------- |---------:|---------:|---------:|------------ |
|      EncodeSum |   0.9.0 |     Small | 93.50 us | 1.839 us | 1.888 us |        8576 |
| EncodeSumField |   0.9.0 |     Small | 95.83 us | 1.908 us | 2.856 us |        9396 |
|  EncodeSumFlag |   0.9.0 |     Small | 90.87 us | 1.404 us | 1.096 us |        8616 |

EncodeSumFlag uses...

message SumFlag {
  repeated NumberDataPoint data_points = 1;

  // aggregation_temporality describes if the aggregator reports delta changes
  // since last report time, or cumulative changes since a fixed start time.
  AggregationTemporality aggregation_temporality = 2;

  uint32 flag = 3;
}

Bit flags are set as boolean flags...

                       sum.Flag = 
                        (1<<0) | 
                        (1<<1) | 
                        (1<<2) | 
                        (1<<3) | 
                        (1<<4) | 
                        (1<<5) | 
                        (1<<6) | 
                        (1<<7) | 
                        (1<<8) | 
                        (1<<9) | 
                        (1<<10) | 
                        (1<<11) | 
                        (1<<12) | 
                        (1<<13) | 
                        (1<<14) | 
                        (1<<15) | 
                        (1<<16) | 
                        (1<<17) | 
                        (1<<18) | 
                        (1<<19) | 
                        (1<<20) | 
                        (1<<21) | 
                        (1<<22) | 
                        (1<<23) | 
                        (1<<24) | 
                        (1<<25) | 
                        (1<<26) | 
                        (1<<27) | 
                        (1<<28) | 
                        (1<<29);

EncodeSumField uses...

message SumField {
  repeated NumberDataPoint data_points = 1;

  // aggregation_temporality describes if the aggregator reports delta changes
  // since last report time, or cumulative changes since a fixed start time.
  AggregationTemporality aggregation_temporality = 2;

  bool isFlag0 = 10;
  bool isFlag1 = 11;
  bool isFlag2 = 12;
  bool isFlag3 = 13;
  bool isFlag4 = 14;
  bool isFlag5 = 15;
  bool isFlag6 = 16;
  bool isFlag7 = 17;
  bool isFlag8 = 18;
  bool isFlag9 = 19;
  bool isFlag10 = 20;
  bool isFlag11 = 21;
  bool isFlag12 = 22;
  bool isFlag13 = 23;
  bool isFlag14 = 24;
  bool isFlag15 = 25;
  bool isFlag16 = 26;
  bool isFlag17 = 27;
  bool isFlag18 = 28;
  bool isFlag19 = 29;
  bool isFlag20 = 30;
  bool isFlag21 = 31;
  bool isFlag22 = 32;
  bool isFlag23 = 33;
  bool isFlag24 = 34;
  bool isFlag25 = 35;
  bool isFlag26 = 36;
  bool isFlag27 = 37;
  bool isFlag28 = 38;
  bool isFlag29 = 39;
  bool isFlag30 = 40;
  bool isFlag31 = 41;
}

Setting Fields...

                    sum.IsFlag0 = true;
                    sum.IsFlag1 = true;
                    sum.IsFlag2 = true;
                    sum.IsFlag3 = true;
                    sum.IsFlag4 = true;
                    sum.IsFlag5 = true;
                    sum.IsFlag6 = true;
                    sum.IsFlag7 = true;
                    sum.IsFlag8 = true;
                    sum.IsFlag9 = true;
                    sum.IsFlag10 = true;
                    sum.IsFlag11 = true;
                    sum.IsFlag12 = true;
                    sum.IsFlag13 = true;
                    sum.IsFlag14 = true;
                    sum.IsFlag15 = true;
                    sum.IsFlag16 = true;
                    sum.IsFlag17 = true;
                    sum.IsFlag18 = true;
                    sum.IsFlag19 = true;
                    sum.IsFlag20 = true;
                    sum.IsFlag21 = true;
                    sum.IsFlag22 = true;
                    sum.IsFlag23 = true;
                    sum.IsFlag24 = true;
                    sum.IsFlag25 = true;
                    sum.IsFlag26 = true;
                    sum.IsFlag27 = true;
                    sum.IsFlag28 = true;
                    sum.IsFlag29 = true;

[victlu]

Adding a uint32 flags field into NumberDataPoint, SummaryDataPoint, and HistogramDataPoint will increase payload size by 6%.

BenchmarkDotNet=v0.13.0, OS=Windows 10.0.19043.1023 (21H1/May2021Update)
Intel Core i7-1065G7 CPU 1.30GHz, 1 CPU, 8 logical and 4 physical cores
.NET SDK=5.0.202
  [Host]     : .NET 5.0.5 (5.0.521.16609), X64 RyuJIT
  DefaultJob : .NET 5.0.5 (5.0.521.16609), X64 RyuJIT


|          Method | Version | ConfigSet |      Mean |    Error |    StdDev |    Median | MessageSize |
|---------------- |-------- |---------- |----------:|---------:|----------:|----------:|------------ |
|     EncodeGauge |   0.9.0 |     Small | 112.80 us | 3.220 us |  9.341 us | 110.26 us |        9156 |
|     EncodeGauge |    HEAD |     Small | 123.79 us | 2.814 us |  8.029 us | 120.99 us |        9756 |
|       EncodeSum |   0.9.0 |     Small | 103.51 us | 2.045 us |  4.131 us | 101.89 us |        9176 |
|       EncodeSum |    HEAD |     Small | 115.41 us | 2.221 us |  4.005 us | 114.15 us |        9776 |
|   EncodeSummary |   0.9.0 |     Small |  31.57 us | 0.622 us |  1.404 us |  30.89 us |        3116 |
|   EncodeSummary |    HEAD |     Small |  35.01 us | 0.635 us |  1.296 us |  34.72 us |        3236 |
| EncodeHistogram |   0.9.0 |     Small | 108.90 us | 2.164 us |  4.118 us | 107.31 us |        8256 |
| EncodeHistogram |    HEAD |     Small | 119.33 us | 2.350 us |  4.801 us | 117.54 us |        8856 |
|     DecodeGauge |   0.9.0 |     Small | 295.24 us | 7.771 us | 22.912 us | 288.27 us |        9156 |
|     DecodeGauge |    HEAD |     Small | 298.17 us | 5.916 us |  5.244 us | 297.77 us |        9756 |
|       DecodeSum |   0.9.0 |     Small |  61.21 us | 1.127 us |  0.999 us |  60.87 us |        9176 |
|       DecodeSum |    HEAD |     Small |  72.74 us | 1.931 us |  5.634 us |  71.26 us |        9776 |
|   DecodeSummary |   0.9.0 |     Small | 102.50 us | 3.220 us |  9.291 us |  99.81 us |        3116 |
|   DecodeSummary |    HEAD |     Small |  98.36 us | 1.955 us |  4.373 us |  96.77 us |        3236 |
| DecodeHistogram |   0.9.0 |     Small | 345.89 us | 7.918 us | 22.972 us | 338.98 us |        8256 |
| DecodeHistogram |    HEAD |     Small | 341.06 us | 6.723 us |  8.503 us | 339.37 us |        8856 |

[jmacd]

In practice, the payload size would only increase if the field is set. If the field is left unset in most points, the payload size would not increase substantially, right?

[victlu]

You are correct. I confirmed with benchmarks as well.

[victlu]

A few questions...

1. We don't have a Version AFAIK in the protocol. Should we introduce a semantic version? This will allow us to clearly designate how to better interpret existing fields and contexts going forward.

2. Should we introduce this flag to all datapoints (i.e. NumberDataPoint, SummaryDataPoint, HistogramDataPoint)?

3. For Histogram and FLAG_NO_RECORDED_VALUE... For backward compatibility (without version in protocol), we can set count = 0. We will need to test this with all existing components.

[tigrannajaryan]

1. We don't have a Version AFAIK in the protocol. Should we introduce a semantic version? This will allow us to clearly designate how to better interpret existing fields and contexts going forward.

See https://github.com/open-telemetry/oteps/blob/main/text/0035-opentelemetry-protocol.md#future-versions-and-interoperability

[victlu]

Proposal for new flags...

FLAG_NON_NEGATIVE_VALUES

FLAG_NON_NEGATIVE_VALUES to help designate a histogram to be non-negative (so to remain compatible with OpenMetrics histograms).

• deprecating current count and sum field in ~3 months.

  • Older code cannot distinguish between sum = 0 and not setting sum. Currently the note says to not set sum if there are negative values in the population.

• introduced population_count and population_sum that honors the new FLAG_NON_NEGATIVE_VALUES in flag field.

  • population_sum will be the sum (including negative values), but can check the FLAG_NON_NEGATIVE_VALUES bit to maintain compatibility with OpenMetrics.

For backward compatibility, as new code sets population_[count/sum] exclusively, the deprecated count and sum to be unset and expected to default to 0. Older code should thus treat this histogram as benign (aka, no data, etc). This statement needs to be validated

FLAG_NO_RECORDED_VALUE

For backward compatibility, older code will not know to check the flag so it cannot distinguish between no-recorded-value vs recorded value but with 0 data points. For older code, we rely on the deprecated count and sum to be unset and thus default to 0. Older code should thus treat this histogram as benign (aka, no data, etc). This statement needs to be validated.

[jmacd]

Note PR #310 included both of these. We are interested in FLAG_NO_RECORDED_VALUE.