Skip to content

Commit 15b7c86

Browse files
authored
[receiver/ntpreceiver] fix ntp.host was not being emitted (open-telemetry#43132)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR fixes the `ntp.host` resource attribute was not being emitted even though it was enabled by default. I'm also proposing being an additional code owner for this component. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#43129 <!--Describe what testing was performed and which tests were added.--> #### Testing I added new tests for `receiver.go`. Also tested it locally with the same configuration shared on the issue. ```sh 2025-10-05T00:16:24.452+0100 info [email protected]/service.go:222 Starting otelcontribcol... {"resource": {"service.instance.id": "9202fda2-095d-4055-8412-dc4de4b8308d", "service.name": "otelcontribcol", "service.version": "0.136.0-dev"}, "Version": "0.136.0-dev", "NumCPU": 12} 2025-10-05T00:16:24.452+0100 info extensions/extensions.go:41 Starting extensions... {"resource": {"service.instance.id": "9202fda2-095d-4055-8412-dc4de4b8308d", "service.name": "otelcontribcol", "service.version": "0.136.0-dev"}} 2025-10-05T00:16:24.452+0100 info [email protected]/service.go:245 Everything is ready. Begin running and processing data. {"resource": {"service.instance.id": "9202fda2-095d-4055-8412-dc4de4b8308d", "service.name": "otelcontribcol", "service.version": "0.136.0-dev"}} 2025-10-05T00:16:25.658+0100 info Metrics {"resource": {"service.instance.id": "9202fda2-095d-4055-8412-dc4de4b8308d", "service.name": "otelcontribcol", "service.version": "0.136.0-dev"}, "otelcol.component.id": "debug", "otelcol.component.kind": "exporter", "otelcol.signal": "metrics", "resource metrics": 1, "metrics": 1, "data points": 1} 2025-10-05T00:16:25.659+0100 info ResourceMetrics #0 Resource SchemaURL: Resource attributes: -> ntp.host: Str(time.nist.gov:123) ScopeMetrics #0 ScopeMetrics SchemaURL: InstrumentationScope github.com/open-telemetry/opentelemetry-collector-contrib/receiver/ntpreceiver 0.136.0-dev Metric #0 Descriptor: -> Name: ntp.offset -> Description: Time difference between local and NTP server clocks -> Unit: ns -> DataType: Gauge NumberDataPoints #0 StartTimestamp: 2025-10-04 23:16:24.452351 +0000 UTC Timestamp: 2025-10-04 23:16:25.647124 +0000 UTC Value: 91327175 {"resource": {"service.instance.id": "9202fda2-095d-4055-8412-dc4de4b8308d", "service.name": "otelcontribcol", "service.version": "0.136.0-dev"}, "otelcol.component.id": "debug", "otelcol.component.kind": "exporter", "otelcol.signal": "metrics"} ``` --------- Signed-off-by: Paulo Dias <[email protected]>
1 parent 5806cfe commit 15b7c86

File tree

6 files changed

+150
-6
lines changed

6 files changed

+150
-6
lines changed

.chloggen/fix_43129.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: "bug_fix"
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: "ntpreceiver"
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: "Fix missing resource attribute 'ntp.host' to ntpreceiver metrics"
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [43129]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]

.github/CODEOWNERS

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ receiver/namedpipereceiver/ @open-telemetry
292292
receiver/netflowreceiver/ @open-telemetry/collector-contrib-approvers @evan-bradley @dlopes7
293293
receiver/nginxreceiver/ @open-telemetry/collector-contrib-approvers @colelaven @ishleenk17
294294
receiver/nsxtreceiver/ @open-telemetry/collector-contrib-approvers @dashpole @schmikei
295-
receiver/ntpreceiver/ @open-telemetry/collector-contrib-approvers @atoulme
295+
receiver/ntpreceiver/ @open-telemetry/collector-contrib-approvers @atoulme @paulojmdias
296296
receiver/oracledbreceiver/ @open-telemetry/collector-contrib-approvers @dmitryax @crobert-1 @atoulme
297297
receiver/osqueryreceiver/ @open-telemetry/collector-contrib-approvers @nslaughter @smithclay
298298
receiver/otelarrowreceiver/ @open-telemetry/collector-contrib-approvers @jmacd @moh-osman3

receiver/ntpreceiver/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
| Distributions | [contrib] |
88
| Issues | [![Open issues](https://img.shields.io/github/issues-search/open-telemetry/opentelemetry-collector-contrib?query=is%3Aissue%20is%3Aopen%20label%3Areceiver%2Fntp%20&label=open&color=orange&logo=opentelemetry)](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues?q=is%3Aopen+is%3Aissue+label%3Areceiver%2Fntp) [![Closed issues](https://img.shields.io/github/issues-search/open-telemetry/opentelemetry-collector-contrib?query=is%3Aissue%20is%3Aclosed%20label%3Areceiver%2Fntp%20&label=closed&color=blue&logo=opentelemetry)](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues?q=is%3Aclosed+is%3Aissue+label%3Areceiver%2Fntp) |
99
| Code coverage | [![codecov](https://codecov.io/github/open-telemetry/opentelemetry-collector-contrib/graph/main/badge.svg?component=receiver_ntp)](https://app.codecov.io/gh/open-telemetry/opentelemetry-collector-contrib/tree/main/?components%5B0%5D=receiver_ntp&displayType=list) |
10-
| [Code Owners](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#becoming-a-code-owner) | [@atoulme](https://www.github.com/atoulme) \| Seeking more code owners! |
10+
| [Code Owners](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#becoming-a-code-owner) | [@atoulme](https://www.github.com/atoulme), [@paulojmdias](https://www.github.com/paulojmdias) \| Seeking more code owners! |
1111

1212
[beta]: https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/component-stability.md#beta
1313
[contrib]: https://github.com/open-telemetry/opentelemetry-collector-releases/tree/main/distributions/otelcol-contrib

receiver/ntpreceiver/metadata.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ status:
66
beta: [metrics]
77
distributions: [contrib]
88
codeowners:
9-
active: [atoulme]
9+
active: [atoulme, paulojmdias]
1010
seeking_new: true
1111

1212
resource_attributes:

receiver/ntpreceiver/receiver.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/ntpreceiver/internal/metadata"
1717
)
1818

19+
var queryWithOptions = ntp.QueryWithOptions
20+
1921
type ntpScraper struct {
2022
logger *zap.Logger
2123
mb *metadata.MetricsBuilder
@@ -26,12 +28,16 @@ type ntpScraper struct {
2628

2729
func (s *ntpScraper) scrape(context.Context) (pmetric.Metrics, error) {
2830
options := ntp.QueryOptions{Version: s.version, Timeout: s.timeout}
29-
response, err := ntp.QueryWithOptions(s.endpoint, options)
31+
response, err := queryWithOptions(s.endpoint, options)
3032
if err != nil {
31-
return pmetric.Metrics{}, err
33+
return pmetric.NewMetrics(), err
3234
}
3335
s.mb.RecordNtpOffsetDataPoint(pcommon.NewTimestampFromTime(time.Now()), response.ClockOffset.Nanoseconds())
34-
s.mb.NewResourceBuilder().SetNtpHost(s.endpoint)
36+
37+
rb := s.mb.NewResourceBuilder()
38+
rb.SetNtpHost(s.endpoint)
39+
40+
s.mb.EmitForResource(metadata.WithResource(rb.Emit()))
3541
return s.mb.Emit(), nil
3642
}
3743

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package ntpreceiver
5+
6+
import (
7+
"errors"
8+
"testing"
9+
"time"
10+
11+
"github.com/beevik/ntp"
12+
"github.com/stretchr/testify/require"
13+
"go.opentelemetry.io/collector/pdata/pmetric"
14+
"go.opentelemetry.io/collector/receiver/receivertest"
15+
16+
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/ntpreceiver/internal/metadata"
17+
)
18+
19+
// Minimal scraper builder for tests (no controller). We keep this close to prod.
20+
func newTestScraper(cfg *Config) *ntpScraper {
21+
settings := receivertest.NewNopSettings(metadata.Type)
22+
return &ntpScraper{
23+
logger: settings.Logger,
24+
mb: metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings),
25+
version: cfg.Version,
26+
endpoint: cfg.Endpoint,
27+
timeout: 5 * time.Second, // fixed timeout for tests
28+
}
29+
}
30+
31+
func TestScraper_Success(t *testing.T) {
32+
// Mock NTP query
33+
old := queryWithOptions
34+
defer func() { queryWithOptions = old }()
35+
queryWithOptions = func(_ string, _ ntp.QueryOptions) (*ntp.Response, error) {
36+
// We return a non-zero offset; test will not assert the value to avoid controller-specific init.
37+
return &ntp.Response{ClockOffset: 250 * time.Millisecond}, nil
38+
}
39+
40+
cfg := createDefaultConfig().(*Config)
41+
cfg.Endpoint = "time.example.com:123"
42+
43+
s := newTestScraper(cfg)
44+
metrics, err := s.scrape(t.Context())
45+
require.NoError(t, err)
46+
require.NotNil(t, metrics)
47+
48+
// ResourceMetrics present
49+
rmSlice := metrics.ResourceMetrics()
50+
require.Equal(t, 1, rmSlice.Len())
51+
52+
// Resource attribute ntp.host set to endpoint
53+
rm := rmSlice.At(0)
54+
resAttrs := rm.Resource().Attributes()
55+
val, ok := resAttrs.Get("ntp.host")
56+
require.True(t, ok, "expected ntp.host resource attribute")
57+
require.Equal(t, cfg.Endpoint, val.Str())
58+
59+
// One scope, one metric
60+
smSlice := rm.ScopeMetrics()
61+
require.Equal(t, 1, smSlice.Len())
62+
mSlice := smSlice.At(0).Metrics()
63+
require.Equal(t, 1, mSlice.Len())
64+
65+
m := mSlice.At(0)
66+
require.Equal(t, "ntp.offset", m.Name())
67+
68+
// Unit should be "ns" per current receiver metadata (do not fail hard if empty, but assert when present)
69+
if u := m.Unit(); u != "" {
70+
require.Equal(t, "ns", u, "expected ntp.offset unit to be ns")
71+
}
72+
73+
// Handle Gauge vs Sum defensively; generator may emit Gauge.
74+
var dps pmetric.NumberDataPointSlice
75+
switch m.Type() {
76+
case pmetric.MetricTypeGauge:
77+
dps = m.Gauge().DataPoints()
78+
case pmetric.MetricTypeSum:
79+
dps = m.Sum().DataPoints()
80+
default:
81+
t.Fatalf("unexpected metric type: %v", m.Type())
82+
}
83+
84+
// Exactly one data point with non-zero timestamps
85+
require.Equal(t, 1, dps.Len())
86+
dp := dps.At(0)
87+
require.NotZero(t, dp.Timestamp(), "datapoint must have a timestamp")
88+
// StartTimestamp may be zero for Gauge; only assert when present
89+
if st := dp.StartTimestamp(); st != 0 {
90+
require.NotZero(t, st, "start timestamp should not be zero when set")
91+
}
92+
}
93+
94+
func TestScraper_Error(t *testing.T) {
95+
// Mock failure
96+
old := queryWithOptions
97+
defer func() { queryWithOptions = old }()
98+
queryWithOptions = func(_ string, _ ntp.QueryOptions) (*ntp.Response, error) {
99+
return nil, errors.New("mock failure")
100+
}
101+
102+
cfg := createDefaultConfig().(*Config)
103+
cfg.Endpoint = "bad.host"
104+
105+
s := newTestScraper(cfg)
106+
metrics, err := s.scrape(t.Context())
107+
108+
require.Error(t, err)
109+
require.NotNil(t, metrics)
110+
require.Equal(t, 0, metrics.ResourceMetrics().Len(), "expected no metrics on error")
111+
}

0 commit comments

Comments
 (0)